linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] reset: npcm: add NPCM reset driver support
@ 2019-10-28 15:54 Tomer Maimon
  2019-10-28 15:54 ` [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation Tomer Maimon
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Tomer Maimon @ 2019-10-28 15:54 UTC (permalink / raw)
  To: p.zabel, robh+dt, mark.rutland, yuenn, venture, benjaminfair,
	avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree, Tomer Maimon

This patch set adds reset controller support 
for the Nuvoton NPCM Baseboard Management Controller (BMC).

Apart of controlling all NPCM BMC reset module lines the NPCM reset driver
support NPCM BMC software reset to restarting the NPCM BMC.

Supporting NPCM USB-PHY reset as follow:

NPCM BMC USB-PHY connected to two modules USB device (UDC) and USB host.

If we will restart the USB-PHY at the UDC probe and later the 
USB host probe will restart USB-PHY again it will disable the UDC
and vice versa.

The solution is to reset the USB-PHY at the reset probe stage before 
the UDC and the USB host are initializing.

NPCM reset driver tested on NPCM750 evaluation board.

Addressed comments from:.
 - kbuild test robot : https://lkml.org/lkml/2019/10/27/768 
  
Changes since version 1:
 - Check if gcr_regmap parameter initialized before using it.

Tomer Maimon (3):
  dt-binding: reset: add NPCM reset controller documentation
  dt-bindings: reset: Add binding constants for NPCM7xx reset controller
  reset: npcm: add NPCM reset controller driver

 .../bindings/reset/nuvoton,npcm-reset.txt     |  35 +++
 drivers/reset/Kconfig                         |   7 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-npcm.c                    | 275 ++++++++++++++++++
 .../dt-bindings/reset/nuvoton,npcm7xx-reset.h |  82 ++++++
 5 files changed, 400 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
 create mode 100644 drivers/reset/reset-npcm.c
 create mode 100644 include/dt-bindings/reset/nuvoton,npcm7xx-reset.h

-- 
2.22.0


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

* [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation
  2019-10-28 15:54 [PATCH v2 0/3] reset: npcm: add NPCM reset driver support Tomer Maimon
@ 2019-10-28 15:54 ` Tomer Maimon
  2019-10-29 15:14   ` Philipp Zabel
  2019-10-28 15:54 ` [PATCH v2 2/3] dt-bindings: reset: Add binding constants for NPCM7xx reset controller Tomer Maimon
  2019-10-28 15:54 ` [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver Tomer Maimon
  2 siblings, 1 reply; 8+ messages in thread
From: Tomer Maimon @ 2019-10-28 15:54 UTC (permalink / raw)
  To: p.zabel, robh+dt, mark.rutland, yuenn, venture, benjaminfair,
	avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree, Tomer Maimon

Added device tree binding documentation for Nuvoton BMC
NPCM reset controller.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../bindings/reset/nuvoton,npcm-reset.txt     | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt

diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
new file mode 100644
index 000000000000..94793285a2ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
@@ -0,0 +1,35 @@
+Nuvoton NPCM Reset controller
+
+In the NPCM Reset controller boot the USB PHY, USB host
+and USB device initialize.
+
+Required properties:
+- compatible : "nuvoton,npcm750-reset" for NPCM7XX BMC
+- reg : specifies physical base address and size of the register.
+- #reset-cells: must be set to 1
+
+Optional property:
+- nuvoton,sw-reset-number - Contains the software reset number to restart the SoC.
+  NPCM7xx contain four software reset that represent numbers 1 to 4.
+
+  If 'nuvoton,sw-reset-number' is not specfied software reset is disabled.
+
+Example:
+	rstc: rstc@f0801000 {
+		compatible = "nuvoton,npcm750-reset";
+		reg = <0xf0801000 0x70>;
+		#reset-cells = <1>;
+		nuvoton,sw-reset-number = <2>;
+	};
+
+Specifying reset lines connected to IP NPCM7XX modules
+======================================================
+example:
+
+        spi0: spi@..... {
+                ...
+                resets = <&rstc NPCM7XX_RESET_PSPI1>;
+                ...
+        };
+
+The index could be found in <dt-bindings/reset/nuvoton,npcm7xx-reset.h>.
-- 
2.22.0


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

* [PATCH v2 2/3] dt-bindings: reset: Add binding constants for NPCM7xx reset controller
  2019-10-28 15:54 [PATCH v2 0/3] reset: npcm: add NPCM reset driver support Tomer Maimon
  2019-10-28 15:54 ` [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation Tomer Maimon
@ 2019-10-28 15:54 ` Tomer Maimon
  2019-10-29 15:15   ` Philipp Zabel
  2019-10-28 15:54 ` [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver Tomer Maimon
  2 siblings, 1 reply; 8+ messages in thread
From: Tomer Maimon @ 2019-10-28 15:54 UTC (permalink / raw)
  To: p.zabel, robh+dt, mark.rutland, yuenn, venture, benjaminfair,
	avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree, Tomer Maimon

Add device tree binding constants for Nuvoton BMC NPCM7xx
reset controller.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../dt-bindings/reset/nuvoton,npcm7xx-reset.h | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 include/dt-bindings/reset/nuvoton,npcm7xx-reset.h

diff --git a/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h
new file mode 100644
index 000000000000..7b7e870eac35
--- /dev/null
+++ b/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copyright (c) 2019 Nuvoton Technology corporation.
+
+#ifndef _DT_BINDINGS_NPCM7XX_RESET_H
+#define _DT_BINDINGS_NPCM7XX_RESET_H
+
+#define NPCM7XX_RESET_FIU3		1
+#define NPCM7XX_RESET_UDC1		5
+#define NPCM7XX_RESET_EMC1		6
+#define NPCM7XX_RESET_UART_2_3		7
+#define NPCM7XX_RESET_UDC2		8
+#define NPCM7XX_RESET_PECI		9
+#define NPCM7XX_RESET_AES		10
+#define NPCM7XX_RESET_UART_0_1		11
+#define NPCM7XX_RESET_MC		12
+#define NPCM7XX_RESET_SMB2		13
+#define NPCM7XX_RESET_SMB3		14
+#define NPCM7XX_RESET_SMB4		15
+#define NPCM7XX_RESET_SMB5		16
+#define NPCM7XX_RESET_PWM_M0		18
+#define NPCM7XX_RESET_TIMER_0_4		19
+#define NPCM7XX_RESET_TIMER_5_9		20
+#define NPCM7XX_RESET_EMC2		21
+#define NPCM7XX_RESET_UDC4		22
+#define NPCM7XX_RESET_UDC5		23
+#define NPCM7XX_RESET_UDC6		24
+#define NPCM7XX_RESET_UDC3		25
+#define NPCM7XX_RESET_ADC		27
+#define NPCM7XX_RESET_SMB6		28
+#define NPCM7XX_RESET_SMB7		29
+#define NPCM7XX_RESET_SMB0		30
+#define NPCM7XX_RESET_SMB1		31
+#define NPCM7XX_RESET_MFT0		32
+#define NPCM7XX_RESET_MFT1		33
+#define NPCM7XX_RESET_MFT2		34
+#define NPCM7XX_RESET_MFT3		35
+#define NPCM7XX_RESET_MFT4		36
+#define NPCM7XX_RESET_MFT5		37
+#define NPCM7XX_RESET_MFT6		38
+#define NPCM7XX_RESET_MFT7		39
+#define NPCM7XX_RESET_MMC		40
+#define NPCM7XX_RESET_SDHC		41
+#define NPCM7XX_RESET_GFX_SYS		42
+#define NPCM7XX_RESET_AHB_PCIBRG	43
+#define NPCM7XX_RESET_VDMA		44
+#define NPCM7XX_RESET_ECE		45
+#define NPCM7XX_RESET_VCD		46
+#define NPCM7XX_RESET_OTP		48
+#define NPCM7XX_RESET_SIOX1		50
+#define NPCM7XX_RESET_SIOX2		51
+#define NPCM7XX_RESET_3DES		53
+#define NPCM7XX_RESET_PSPI1		54
+#define NPCM7XX_RESET_PSPI2		55
+#define NPCM7XX_RESET_GMAC2		57
+#define NPCM7XX_RESET_USB_HOST		58
+#define NPCM7XX_RESET_GMAC1		60
+#define NPCM7XX_RESET_CP		63
+#define NPCM7XX_RESET_PWM_M1		160
+#define NPCM7XX_RESET_SMB12		161
+#define NPCM7XX_RESET_SPIX		162
+#define NPCM7XX_RESET_SMB13		163
+#define NPCM7XX_RESET_UDC0		164
+#define NPCM7XX_RESET_UDC7		165
+#define NPCM7XX_RESET_UDC8		166
+#define NPCM7XX_RESET_UDC9		167
+#define NPCM7XX_RESET_PCI_MAILBOX	169
+#define NPCM7XX_RESET_SMB14		172
+#define NPCM7XX_RESET_SHA		173
+#define NPCM7XX_RESET_SEC_ECC		174
+#define NPCM7XX_RESET_PCIE_RC		175
+#define NPCM7XX_RESET_TIMER_10_14	176
+#define NPCM7XX_RESET_RNG		177
+#define NPCM7XX_RESET_SMB15		178
+#define NPCM7XX_RESET_SMB8		179
+#define NPCM7XX_RESET_SMB9		180
+#define NPCM7XX_RESET_SMB10		181
+#define NPCM7XX_RESET_SMB11		182
+#define NPCM7XX_RESET_ESPI		183
+#define NPCM7XX_RESET_USB_PHY_1		184
+#define NPCM7XX_RESET_USB_PHY_2		185
+
+#endif
-- 
2.22.0


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

* [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver
  2019-10-28 15:54 [PATCH v2 0/3] reset: npcm: add NPCM reset driver support Tomer Maimon
  2019-10-28 15:54 ` [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation Tomer Maimon
  2019-10-28 15:54 ` [PATCH v2 2/3] dt-bindings: reset: Add binding constants for NPCM7xx reset controller Tomer Maimon
@ 2019-10-28 15:54 ` Tomer Maimon
  2019-10-29 15:34   ` Philipp Zabel
  2019-10-30  5:11   ` kbuild test robot
  2 siblings, 2 replies; 8+ messages in thread
From: Tomer Maimon @ 2019-10-28 15:54 UTC (permalink / raw)
  To: p.zabel, robh+dt, mark.rutland, yuenn, venture, benjaminfair,
	avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree, Tomer Maimon

Add Nuvoton NPCM BMC reset controller driver.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/reset/Kconfig      |   7 +
 drivers/reset/Makefile     |   1 +
 drivers/reset/reset-npcm.c | 275 +++++++++++++++++++++++++++++++++++++
 3 files changed, 283 insertions(+)
 create mode 100644 drivers/reset/reset-npcm.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 7b07281aa0ae..5dbfdf6d717a 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -89,6 +89,13 @@ config RESET_MESON_AUDIO_ARB
 	  This enables the reset driver for Audio Memory Arbiter of
 	  Amlogic's A113 based SoCs
 
+config RESET_NPCM
+	bool "NPCM BMC Reset Driver"
+	depends on ARCH_NPCM || COMPILE_TEST
+	help
+	  This enables the reset controller driver for Nuvoton NPCM 
+	  BMC SoCs.
+
 config RESET_OXNAS
 	bool
 
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index cf60ce526064..00767c03f5f2 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
 obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
+obj-$(CONFIG_RESET_NPCM) += reset-npcm.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
new file mode 100644
index 000000000000..ebb3071767e1
--- /dev/null
+++ b/drivers/reset/reset-npcm.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Nuvoton Technology corporation.
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/of_address.h>
+
+/* NPCM7xx GCR registers */
+#define NPCM_MDLR_OFFSET	0x7C
+#define NPCM_MDLR_USBD0		BIT(9)
+#define NPCM_MDLR_USBD1		BIT(8)
+#define NPCM_MDLR_USBD2_4	BIT(21)
+#define NPCM_MDLR_USBD5_9	BIT(22)
+
+#define NPCM_USB1PHYCTL_OFFSET	0x140
+#define NPCM_USB2PHYCTL_OFFSET	0x144
+#define NPCM_USBXPHYCTL_RS	BIT(28)
+
+/* NPCM7xx Reset registers */
+#define NPCM_SWRSTR		0x14
+#define NPCM_SWRST		BIT(2)
+
+#define NPCM_IPSRST1		0x20
+#define NPCM_IPSRST1_USBD1	BIT(5)
+#define NPCM_IPSRST1_USBD2	BIT(8)
+#define NPCM_IPSRST1_USBD3	BIT(25)
+#define NPCM_IPSRST1_USBD4	BIT(22)
+#define NPCM_IPSRST1_USBD5	BIT(23)
+#define NPCM_IPSRST1_USBD6	BIT(24)
+
+#define NPCM_IPSRST2		0x24
+#define NPCM_IPSRST2_USB_HOST	BIT(26)
+
+#define NPCM_IPSRST3		0x34
+#define NPCM_IPSRST3_USBD0	BIT(4)
+#define NPCM_IPSRST3_USBD7	BIT(5)
+#define NPCM_IPSRST3_USBD8	BIT(6)
+#define NPCM_IPSRST3_USBD9	BIT(7)
+#define NPCM_IPSRST3_USBPHY1	BIT(24)
+#define NPCM_IPSRST3_USBPHY2	BIT(25)
+
+#define NPCM_RC_RESETS_PER_REG	32
+
+struct npcm_rc_data {
+	struct reset_controller_dev rcdev;
+	struct notifier_block restart_nb;
+	u32 sw_reset_number;
+	void __iomem *base;
+	spinlock_t lock;
+};
+
+#define to_rc_data(p) container_of(p, struct npcm_rc_data, rcdev)
+
+static int npcm_rc_restart(struct notifier_block *nb, unsigned long mode,
+			   void *cmd)
+{
+	struct npcm_rc_data *rc = container_of(nb, struct npcm_rc_data,
+					       restart_nb);
+
+	writel(NPCM_SWRST << rc->sw_reset_number, rc->base + NPCM_SWRSTR);
+	mdelay(1000);
+
+	pr_emerg("%s: unable to restart system\n", __func__);
+
+	return NOTIFY_DONE;
+}
+
+static int npcm_rc_setclear_reset(struct reset_controller_dev *rcdev,
+				  unsigned long id, bool set)
+{
+	struct npcm_rc_data *rc = to_rc_data(rcdev);
+	u32 ctrl_offset = NPCM_IPSRST1;
+	unsigned long flags;
+	u32 stat, rst_bit;
+
+	ctrl_offset += (id / NPCM_RC_RESETS_PER_REG) * sizeof(u32);
+	rst_bit = 1 << (id % NPCM_RC_RESETS_PER_REG);
+
+	spin_lock_irqsave(&rc->lock, flags);
+	stat = readl(rc->base + ctrl_offset);
+	if (set)
+		writel(stat | rst_bit, rc->base + ctrl_offset);
+	else
+		writel(stat & ~rst_bit, rc->base + ctrl_offset);
+	spin_unlock_irqrestore(&rc->lock, flags);
+
+	return 0;
+}
+
+static int npcm_rc_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	return npcm_rc_setclear_reset(rcdev, id, true);
+}
+
+static int npcm_rc_deassert(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	return npcm_rc_setclear_reset(rcdev, id, false);
+}
+
+static int npcm_rc_status(struct reset_controller_dev *rcdev,
+			  unsigned long id)
+{
+	struct npcm_rc_data *rc = to_rc_data(rcdev);
+	u32 bit, ctrl_offset = NPCM_IPSRST1;
+
+	ctrl_offset += (id / NPCM_RC_RESETS_PER_REG) * sizeof(u32);
+	bit = 1 << (id % NPCM_RC_RESETS_PER_REG);
+
+	return (readl(rc->base + ctrl_offset) & bit);
+}
+
+/*
+ *  The following procedure should be observed in USB PHY, USB device and
+ *  USB host initialization at BMC boot
+ */
+static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
+{
+	struct device_node *np = pdev->dev.of_node;
+	u32 mdlr, iprst1, iprst2, iprst3;
+	struct regmap *gcr_regmap;
+	u32 ipsrst1_bits = 0;
+	u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
+	u32 ipsrst3_bits = 0;
+
+	if (of_device_is_compatible(np, "nuvoton,npcm750-reset")) {
+		gcr_regmap = syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
+		if (IS_ERR(gcr_regmap)) {
+			dev_err(&pdev->dev, "Failed to find nuvoton,npcm750-gcr\n");
+			return PTR_ERR(gcr_regmap);
+		}
+	}
+	if (!gcr_regmap)
+		return -ENXIO;
+
+	/* checking which USB device is enabled */
+	regmap_read(gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
+	if (!(mdlr & NPCM_MDLR_USBD0))
+		ipsrst3_bits |= NPCM_IPSRST3_USBD0;
+	if (!(mdlr & NPCM_MDLR_USBD1))
+		ipsrst1_bits |= NPCM_IPSRST1_USBD1;
+	if (!(mdlr & NPCM_MDLR_USBD2_4))
+		ipsrst1_bits |= (NPCM_IPSRST1_USBD2 |
+				 NPCM_IPSRST1_USBD3 |
+				 NPCM_IPSRST1_USBD4);
+	if (!(mdlr & NPCM_MDLR_USBD0)) {
+		ipsrst1_bits |= (NPCM_IPSRST1_USBD5 |
+				 NPCM_IPSRST1_USBD6);
+		ipsrst3_bits |= (NPCM_IPSRST3_USBD7 |
+				 NPCM_IPSRST3_USBD8 |
+				 NPCM_IPSRST3_USBD9);
+	}
+
+	/* assert reset USB PHY and USB devices */
+	iprst1 = readl(rc->base + NPCM_IPSRST1);
+	iprst2 = readl(rc->base + NPCM_IPSRST2);
+	iprst3 = readl(rc->base + NPCM_IPSRST3);
+
+	iprst1 |= ipsrst1_bits;
+	iprst2 |= ipsrst2_bits;
+	iprst3 |= (ipsrst3_bits | NPCM_IPSRST3_USBPHY1 |
+		   NPCM_IPSRST3_USBPHY2);
+
+	writel(iprst1, rc->base + NPCM_IPSRST1);
+	writel(iprst2, rc->base + NPCM_IPSRST2);
+	writel(iprst3, rc->base + NPCM_IPSRST3);
+
+	/* clear USB PHY RS bit */
+	regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, 0);
+	regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, 0);
+
+	/* deassert reset USB PHY */
+	iprst3 &= ~(NPCM_IPSRST3_USBPHY1 | NPCM_IPSRST3_USBPHY2);
+	writel(iprst3, rc->base + NPCM_IPSRST3);
+
+	udelay(50);
+
+	/* set USB PHY RS bit */
+	regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
+	regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
+
+	/* deassert reset USB devices*/
+	iprst1 &= ~ipsrst1_bits;
+	iprst2 &= ~ipsrst2_bits;
+	iprst3 &= ~ipsrst3_bits;
+
+	writel(iprst1, rc->base + NPCM_IPSRST1);
+	writel(iprst2, rc->base + NPCM_IPSRST2);
+	writel(iprst3, rc->base + NPCM_IPSRST3);
+
+	return 0;
+}
+
+static const struct reset_control_ops npcm_rc_ops = {
+	.assert		= npcm_rc_assert,
+	.deassert	= npcm_rc_deassert,
+	.status		= npcm_rc_status,
+};
+
+static int npcm_rc_probe(struct platform_device *pdev)
+{
+	struct npcm_rc_data *rc;
+	struct resource res;
+	int ret;
+
+	rc = devm_kzalloc(&pdev->dev, sizeof(*rc), GFP_KERNEL);
+	if (!rc)
+		return -ENOMEM;
+
+	of_address_to_resource(pdev->dev.of_node, 0, &res);
+	rc->base = devm_ioremap_resource(&pdev->dev, &res);
+	if (IS_ERR(rc->base))
+		return PTR_ERR(rc->base);
+
+	spin_lock_init(&rc->lock);
+
+	rc->rcdev.owner = THIS_MODULE;
+	rc->rcdev.nr_resets = resource_size(&res) / 4 * BITS_PER_LONG;
+	rc->rcdev.ops = &npcm_rc_ops;
+	rc->rcdev.of_node = pdev->dev.of_node;
+
+	platform_set_drvdata(pdev, rc);
+
+	ret = reset_controller_register(&rc->rcdev);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to register device\n");
+		return ret;
+	}
+
+	if (npcm_usb_reset(pdev, rc))
+		dev_warn(&pdev->dev, "NPCM USB reset failed, can cause issues with UDC and USB host\n");
+
+	if (!of_property_read_u32(pdev->dev.of_node, "nuvoton,sw-reset-number",
+				  &rc->sw_reset_number)) {
+		if (rc->sw_reset_number && rc->sw_reset_number < 5) {
+			rc->restart_nb.priority = 192,
+			rc->restart_nb.notifier_call = npcm_rc_restart,
+			ret = register_restart_handler(&rc->restart_nb);
+			if (ret)
+				dev_warn(&pdev->dev, "failed to register restart handler\n");
+		}
+	}
+
+	pr_info("NPCM RESET driver probed\n");
+	return ret;
+}
+
+static const struct of_device_id npcm_rc_match[] = {
+	{ .compatible = "nuvoton,npcm750-reset" },
+	{ }
+};
+
+static struct platform_driver npcm_rc_driver = {
+	.probe	= npcm_rc_probe,
+	.driver	= {
+		.name			= "npcm-reset",
+		.of_match_table		= npcm_rc_match,
+		.suppress_bind_attrs	= true,
+	},
+};
+builtin_platform_driver(npcm_rc_driver);
-- 
2.22.0


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

* Re: [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation
  2019-10-28 15:54 ` [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation Tomer Maimon
@ 2019-10-29 15:14   ` Philipp Zabel
  0 siblings, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2019-10-29 15:14 UTC (permalink / raw)
  To: Tomer Maimon, robh+dt, mark.rutland, yuenn, venture,
	benjaminfair, avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree

Hi Tomer,

On Mon, 2019-10-28 at 17:54 +0200, Tomer Maimon wrote:
> Added device tree binding documentation for Nuvoton BMC
> NPCM reset controller.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../bindings/reset/nuvoton,npcm-reset.txt     | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
> new file mode 100644
> index 000000000000..94793285a2ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
> @@ -0,0 +1,35 @@
> +Nuvoton NPCM Reset controller
> +
> +In the NPCM Reset controller boot the USB PHY, USB host
> +and USB device initialize.

Isn't this just a detail of the driver implementation?

> +Required properties:
> +- compatible : "nuvoton,npcm750-reset" for NPCM7XX BMC

Is this driver expected to be reused for other SoCs?

> +- reg : specifies physical base address and size of the register.
> +- #reset-cells: must be set to 1
> +
> +Optional property:
> +- nuvoton,sw-reset-number - Contains the software reset number to restart the SoC.
> +  NPCM7xx contain four software reset that represent numbers 1 to 4.

What's the difference between the four restart bits? Is this something
that has to be configured per board?

> +  If 'nuvoton,sw-reset-number' is not specfied software reset is disabled.
> +
> +Example:
> +	rstc: rstc@f0801000 {
> +		compatible = "nuvoton,npcm750-reset";
> +		reg = <0xf0801000 0x70>;
> +		#reset-cells = <1>;
> +		nuvoton,sw-reset-number = <2>;
> +	};
> +
> +Specifying reset lines connected to IP NPCM7XX modules
> +======================================================
> +example:
> +
> +        spi0: spi@..... {
> +                ...
> +                resets = <&rstc NPCM7XX_RESET_PSPI1>;
> +                ...
> +        };
> +
> +The index could be found in <dt-bindings/reset/nuvoton,npcm7xx-reset.h>.

regards
Philipp


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

* Re: [PATCH v2 2/3] dt-bindings: reset: Add binding constants for NPCM7xx reset controller
  2019-10-28 15:54 ` [PATCH v2 2/3] dt-bindings: reset: Add binding constants for NPCM7xx reset controller Tomer Maimon
@ 2019-10-29 15:15   ` Philipp Zabel
  0 siblings, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2019-10-29 15:15 UTC (permalink / raw)
  To: Tomer Maimon, robh+dt, mark.rutland, yuenn, venture,
	benjaminfair, avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree

On Mon, 2019-10-28 at 17:54 +0200, Tomer Maimon wrote:
> Add device tree binding constants for Nuvoton BMC NPCM7xx
> reset controller.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../dt-bindings/reset/nuvoton,npcm7xx-reset.h | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 include/dt-bindings/reset/nuvoton,npcm7xx-reset.h
> 
> diff --git a/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h
> new file mode 100644
> index 000000000000..7b7e870eac35
> --- /dev/null
> +++ b/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +// Copyright (c) 2019 Nuvoton Technology corporation.
> +
> +#ifndef _DT_BINDINGS_NPCM7XX_RESET_H
> +#define _DT_BINDINGS_NPCM7XX_RESET_H
> +
> +#define NPCM7XX_RESET_FIU3		1
> +#define NPCM7XX_RESET_UDC1		5
> +#define NPCM7XX_RESET_EMC1		6
> +#define NPCM7XX_RESET_UART_2_3		7
> +#define NPCM7XX_RESET_UDC2		8
> +#define NPCM7XX_RESET_PECI		9
> +#define NPCM7XX_RESET_AES		10
> +#define NPCM7XX_RESET_UART_0_1		11
> +#define NPCM7XX_RESET_MC		12
> +#define NPCM7XX_RESET_SMB2		13
> +#define NPCM7XX_RESET_SMB3		14
> +#define NPCM7XX_RESET_SMB4		15
> +#define NPCM7XX_RESET_SMB5		16
> +#define NPCM7XX_RESET_PWM_M0		18
> +#define NPCM7XX_RESET_TIMER_0_4		19
> +#define NPCM7XX_RESET_TIMER_5_9		20
> +#define NPCM7XX_RESET_EMC2		21
> +#define NPCM7XX_RESET_UDC4		22
> +#define NPCM7XX_RESET_UDC5		23
> +#define NPCM7XX_RESET_UDC6		24
> +#define NPCM7XX_RESET_UDC3		25
> +#define NPCM7XX_RESET_ADC		27
> +#define NPCM7XX_RESET_SMB6		28
> +#define NPCM7XX_RESET_SMB7		29
> +#define NPCM7XX_RESET_SMB0		30
> +#define NPCM7XX_RESET_SMB1		31
> +#define NPCM7XX_RESET_MFT0		32
> +#define NPCM7XX_RESET_MFT1		33
> +#define NPCM7XX_RESET_MFT2		34
> +#define NPCM7XX_RESET_MFT3		35
> +#define NPCM7XX_RESET_MFT4		36
> +#define NPCM7XX_RESET_MFT5		37
> +#define NPCM7XX_RESET_MFT6		38
> +#define NPCM7XX_RESET_MFT7		39
> +#define NPCM7XX_RESET_MMC		40
> +#define NPCM7XX_RESET_SDHC		41
> +#define NPCM7XX_RESET_GFX_SYS		42
> +#define NPCM7XX_RESET_AHB_PCIBRG	43
> +#define NPCM7XX_RESET_VDMA		44
> +#define NPCM7XX_RESET_ECE		45
> +#define NPCM7XX_RESET_VCD		46
> +#define NPCM7XX_RESET_OTP		48
> +#define NPCM7XX_RESET_SIOX1		50
> +#define NPCM7XX_RESET_SIOX2		51
> +#define NPCM7XX_RESET_3DES		53
> +#define NPCM7XX_RESET_PSPI1		54
> +#define NPCM7XX_RESET_PSPI2		55
> +#define NPCM7XX_RESET_GMAC2		57
> +#define NPCM7XX_RESET_USB_HOST		58
> +#define NPCM7XX_RESET_GMAC1		60
> +#define NPCM7XX_RESET_CP		63

What's in the gap between IPSRST2 and IPSRST3? Are you sure you don't
want the following IPSRST3 resets to just start at 64? That could be
achieved with a custom of_xlate callback in the driver.

> +#define NPCM7XX_RESET_PWM_M1		160
> +#define NPCM7XX_RESET_SMB12		161
> +#define NPCM7XX_RESET_SPIX		162
> +#define NPCM7XX_RESET_SMB13		163
> +#define NPCM7XX_RESET_UDC0		164
> +#define NPCM7XX_RESET_UDC7		165
> +#define NPCM7XX_RESET_UDC8		166
> +#define NPCM7XX_RESET_UDC9		167
> +#define NPCM7XX_RESET_PCI_MAILBOX	169
> +#define NPCM7XX_RESET_SMB14		172
> +#define NPCM7XX_RESET_SHA		173
> +#define NPCM7XX_RESET_SEC_ECC		174
> +#define NPCM7XX_RESET_PCIE_RC		175
> +#define NPCM7XX_RESET_TIMER_10_14	176
> +#define NPCM7XX_RESET_RNG		177
> +#define NPCM7XX_RESET_SMB15		178
> +#define NPCM7XX_RESET_SMB8		179
> +#define NPCM7XX_RESET_SMB9		180
> +#define NPCM7XX_RESET_SMB10		181
> +#define NPCM7XX_RESET_SMB11		182
> +#define NPCM7XX_RESET_ESPI		183
> +#define NPCM7XX_RESET_USB_PHY_1		184
> +#define NPCM7XX_RESET_USB_PHY_2		185
> +
> +#endif

regards
Philipp


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

* Re: [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver
  2019-10-28 15:54 ` [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver Tomer Maimon
@ 2019-10-29 15:34   ` Philipp Zabel
  2019-10-30  5:11   ` kbuild test robot
  1 sibling, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2019-10-29 15:34 UTC (permalink / raw)
  To: Tomer Maimon, robh+dt, mark.rutland, yuenn, venture,
	benjaminfair, avifishman70, joel
  Cc: openbmc, linux-kernel, devicetree

On Mon, 2019-10-28 at 17:54 +0200, Tomer Maimon wrote:
> Add Nuvoton NPCM BMC reset controller driver.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  drivers/reset/Kconfig      |   7 +
>  drivers/reset/Makefile     |   1 +
>  drivers/reset/reset-npcm.c | 275 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 283 insertions(+)
>  create mode 100644 drivers/reset/reset-npcm.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 7b07281aa0ae..5dbfdf6d717a 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -89,6 +89,13 @@ config RESET_MESON_AUDIO_ARB
>  	  This enables the reset driver for Audio Memory Arbiter of
>  	  Amlogic's A113 based SoCs
>  
> +config RESET_NPCM
> +	bool "NPCM BMC Reset Driver"
> +	depends on ARCH_NPCM || COMPILE_TEST
> +	help
> +	  This enables the reset controller driver for Nuvoton NPCM 
> +	  BMC SoCs.
> +

Is there any reason to ever disable this driver when building ARCH_NPCM?

>  config RESET_OXNAS
>  	bool
>  
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index cf60ce526064..00767c03f5f2 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
> +obj-$(CONFIG_RESET_NPCM) += reset-npcm.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>  obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>  obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
> diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
> new file mode 100644
> index 000000000000..ebb3071767e1
> --- /dev/null
> +++ b/drivers/reset/reset-npcm.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Nuvoton Technology corporation.
> +
> +#include <linux/clk.h>

Please remove unused header includes.

> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/of_address.h>
> +
> +/* NPCM7xx GCR registers */
> +#define NPCM_MDLR_OFFSET	0x7C
> +#define NPCM_MDLR_USBD0		BIT(9)
> +#define NPCM_MDLR_USBD1		BIT(8)
> +#define NPCM_MDLR_USBD2_4	BIT(21)
> +#define NPCM_MDLR_USBD5_9	BIT(22)
> +
> +#define NPCM_USB1PHYCTL_OFFSET	0x140
> +#define NPCM_USB2PHYCTL_OFFSET	0x144
> +#define NPCM_USBXPHYCTL_RS	BIT(28)
> +
> +/* NPCM7xx Reset registers */
> +#define NPCM_SWRSTR		0x14
> +#define NPCM_SWRST		BIT(2)
> +
> +#define NPCM_IPSRST1		0x20
> +#define NPCM_IPSRST1_USBD1	BIT(5)
> +#define NPCM_IPSRST1_USBD2	BIT(8)
> +#define NPCM_IPSRST1_USBD3	BIT(25)
> +#define NPCM_IPSRST1_USBD4	BIT(22)
> +#define NPCM_IPSRST1_USBD5	BIT(23)
> +#define NPCM_IPSRST1_USBD6	BIT(24)
> +
> +#define NPCM_IPSRST2		0x24
> +#define NPCM_IPSRST2_USB_HOST	BIT(26)
> +
> +#define NPCM_IPSRST3		0x34
> +#define NPCM_IPSRST3_USBD0	BIT(4)
> +#define NPCM_IPSRST3_USBD7	BIT(5)
> +#define NPCM_IPSRST3_USBD8	BIT(6)
> +#define NPCM_IPSRST3_USBD9	BIT(7)
> +#define NPCM_IPSRST3_USBPHY1	BIT(24)
> +#define NPCM_IPSRST3_USBPHY2	BIT(25)
> +
> +#define NPCM_RC_RESETS_PER_REG	32
> +
> +struct npcm_rc_data {
> +	struct reset_controller_dev rcdev;
> +	struct notifier_block restart_nb;
> +	u32 sw_reset_number;
> +	void __iomem *base;
> +	spinlock_t lock;
> +};
> +
> +#define to_rc_data(p) container_of(p, struct npcm_rc_data, rcdev)
> +
> +static int npcm_rc_restart(struct notifier_block *nb, unsigned long mode,
> +			   void *cmd)
> +{
> +	struct npcm_rc_data *rc = container_of(nb, struct npcm_rc_data,
> +					       restart_nb);
> +
> +	writel(NPCM_SWRST << rc->sw_reset_number, rc->base + NPCM_SWRSTR);
> +	mdelay(1000);
> +
> +	pr_emerg("%s: unable to restart system\n", __func__);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int npcm_rc_setclear_reset(struct reset_controller_dev *rcdev,
> +				  unsigned long id, bool set)
> +{
> +	struct npcm_rc_data *rc = to_rc_data(rcdev);
> +	u32 ctrl_offset = NPCM_IPSRST1;
> +	unsigned long flags;
> +	u32 stat, rst_bit;
> +
> +	ctrl_offset += (id / NPCM_RC_RESETS_PER_REG) * sizeof(u32);
> +	rst_bit = 1 << (id % NPCM_RC_RESETS_PER_REG);
> +
> +	spin_lock_irqsave(&rc->lock, flags);
> +	stat = readl(rc->base + ctrl_offset);
> +	if (set)
> +		writel(stat | rst_bit, rc->base + ctrl_offset);
> +	else
> +		writel(stat & ~rst_bit, rc->base + ctrl_offset);
> +	spin_unlock_irqrestore(&rc->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int npcm_rc_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	return npcm_rc_setclear_reset(rcdev, id, true);
> +}
> +
> +static int npcm_rc_deassert(struct reset_controller_dev *rcdev,
> +			    unsigned long id)
> +{
> +	return npcm_rc_setclear_reset(rcdev, id, false);
> +}
> +
> +static int npcm_rc_status(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	struct npcm_rc_data *rc = to_rc_data(rcdev);
> +	u32 bit, ctrl_offset = NPCM_IPSRST1;
> +
> +	ctrl_offset += (id / NPCM_RC_RESETS_PER_REG) * sizeof(u32);
> +	bit = 1 << (id % NPCM_RC_RESETS_PER_REG);
> +
> +	return (readl(rc->base + ctrl_offset) & bit);
> +}
> +
> +/*
> + *  The following procedure should be observed in USB PHY, USB device and
> + *  USB host initialization at BMC boot
> + */
> +static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	u32 mdlr, iprst1, iprst2, iprst3;
> +	struct regmap *gcr_regmap;
> +	u32 ipsrst1_bits = 0;
> +	u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
> +	u32 ipsrst3_bits = 0;
> +
> +	if (of_device_is_compatible(np, "nuvoton,npcm750-reset")) {
> +		gcr_regmap = syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
> +		if (IS_ERR(gcr_regmap)) {
> +			dev_err(&pdev->dev, "Failed to find nuvoton,npcm750-gcr\n");
> +			return PTR_ERR(gcr_regmap);
> +		}
> +	}
> +	if (!gcr_regmap)
> +		return -ENXIO;
> +
> +	/* checking which USB device is enabled */
> +	regmap_read(gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
> +	if (!(mdlr & NPCM_MDLR_USBD0))
> +		ipsrst3_bits |= NPCM_IPSRST3_USBD0;
> +	if (!(mdlr & NPCM_MDLR_USBD1))
> +		ipsrst1_bits |= NPCM_IPSRST1_USBD1;
> +	if (!(mdlr & NPCM_MDLR_USBD2_4))
> +		ipsrst1_bits |= (NPCM_IPSRST1_USBD2 |
> +				 NPCM_IPSRST1_USBD3 |
> +				 NPCM_IPSRST1_USBD4);
> +	if (!(mdlr & NPCM_MDLR_USBD0)) {
> +		ipsrst1_bits |= (NPCM_IPSRST1_USBD5 |
> +				 NPCM_IPSRST1_USBD6);
> +		ipsrst3_bits |= (NPCM_IPSRST3_USBD7 |
> +				 NPCM_IPSRST3_USBD8 |
> +				 NPCM_IPSRST3_USBD9);
> +	}
> +
> +	/* assert reset USB PHY and USB devices */
> +	iprst1 = readl(rc->base + NPCM_IPSRST1);
> +	iprst2 = readl(rc->base + NPCM_IPSRST2);
> +	iprst3 = readl(rc->base + NPCM_IPSRST3);
> +
> +	iprst1 |= ipsrst1_bits;
> +	iprst2 |= ipsrst2_bits;
> +	iprst3 |= (ipsrst3_bits | NPCM_IPSRST3_USBPHY1 |
> +		   NPCM_IPSRST3_USBPHY2);
> +
> +	writel(iprst1, rc->base + NPCM_IPSRST1);
> +	writel(iprst2, rc->base + NPCM_IPSRST2);
> +	writel(iprst3, rc->base + NPCM_IPSRST3);
> +
> +	/* clear USB PHY RS bit */
> +	regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
> +			   NPCM_USBXPHYCTL_RS, 0);
> +	regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
> +			   NPCM_USBXPHYCTL_RS, 0);
> +
> +	/* deassert reset USB PHY */
> +	iprst3 &= ~(NPCM_IPSRST3_USBPHY1 | NPCM_IPSRST3_USBPHY2);
> +	writel(iprst3, rc->base + NPCM_IPSRST3);
> +
> +	udelay(50);
> +
> +	/* set USB PHY RS bit */
> +	regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
> +			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
> +	regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
> +			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
> +
> +	/* deassert reset USB devices*/
> +	iprst1 &= ~ipsrst1_bits;
> +	iprst2 &= ~ipsrst2_bits;
> +	iprst3 &= ~ipsrst3_bits;
> +
> +	writel(iprst1, rc->base + NPCM_IPSRST1);
> +	writel(iprst2, rc->base + NPCM_IPSRST2);
> +	writel(iprst3, rc->base + NPCM_IPSRST3);
> +
> +	return 0;
> +}

Is there no better place for this, such as USB glue code?

> +static const struct reset_control_ops npcm_rc_ops = {
> +	.assert		= npcm_rc_assert,
> +	.deassert	= npcm_rc_deassert,
> +	.status		= npcm_rc_status,
> +};
> +
> +static int npcm_rc_probe(struct platform_device *pdev)
> +{
> +	struct npcm_rc_data *rc;
> +	struct resource res;
> +	int ret;
> +
> +	rc = devm_kzalloc(&pdev->dev, sizeof(*rc), GFP_KERNEL);
> +	if (!rc)
> +		return -ENOMEM;
> +
> +	of_address_to_resource(pdev->dev.of_node, 0, &res);
> +	rc->base = devm_ioremap_resource(&pdev->dev, &res);

Can't you just use

	rc->base = devm_platform_ioremap_resource(pdev, 0);

here?

> +	if (IS_ERR(rc->base))
> +		return PTR_ERR(rc->base);
> +
> +	spin_lock_init(&rc->lock);
> +
> +	rc->rcdev.owner = THIS_MODULE;
> +	rc->rcdev.nr_resets = resource_size(&res) / 4 * BITS_PER_LONG;

That doesn't seem right. With the ctrl_offset = NPCM_IPSRST1 in
npcm_rc_setclear_reset that would allow access beyond the configured
register range.

> +	rc->rcdev.ops = &npcm_rc_ops;
> +	rc->rcdev.of_node = pdev->dev.of_node;
> +
> +	platform_set_drvdata(pdev, rc);
> +
> +	ret = reset_controller_register(&rc->rcdev);

	ret = devm_reset_controller_register(&pdev->dev, &rc->rcdev);

> +	if (ret) {
> +		dev_err(&pdev->dev, "unable to register device\n");
> +		return ret;
> +	}
> +
> +	if (npcm_usb_reset(pdev, rc))
> +		dev_warn(&pdev->dev, "NPCM USB reset failed, can cause issues with UDC and USB host\n");
> +
> +	if (!of_property_read_u32(pdev->dev.of_node, "nuvoton,sw-reset-number",
> +				  &rc->sw_reset_number)) {
> +		if (rc->sw_reset_number && rc->sw_reset_number < 5) {
> +			rc->restart_nb.priority = 192,
> +			rc->restart_nb.notifier_call = npcm_rc_restart,
> +			ret = register_restart_handler(&rc->restart_nb);
> +			if (ret)
> +				dev_warn(&pdev->dev, "failed to register restart handler\n");
> +		}
> +	}
> +
> +	pr_info("NPCM RESET driver probed\n");

It think this is a bit verbose.

> +	return ret;
> +}
> +
> +static const struct of_device_id npcm_rc_match[] = {
> +	{ .compatible = "nuvoton,npcm750-reset" },
> +	{ }
> +};
> +
> +static struct platform_driver npcm_rc_driver = {
> +	.probe	= npcm_rc_probe,
> +	.driver	= {
> +		.name			= "npcm-reset",
> +		.of_match_table		= npcm_rc_match,
> +		.suppress_bind_attrs	= true,
> +	},
> +};
> +builtin_platform_driver(npcm_rc_driver);

regards
Philipp


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

* Re: [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver
  2019-10-28 15:54 ` [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver Tomer Maimon
  2019-10-29 15:34   ` Philipp Zabel
@ 2019-10-30  5:11   ` kbuild test robot
  1 sibling, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2019-10-30  5:11 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: kbuild-all, p.zabel, robh+dt, mark.rutland, yuenn, venture,
	benjaminfair, avifishman70, joel, openbmc, linux-kernel,
	devicetree, Tomer Maimon

[-- Attachment #1: Type: text/plain, Size: 4965 bytes --]

Hi Tomer,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pza/reset/next]
[also build test WARNING on v5.4-rc5 next-20191029]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tomer-Maimon/reset-npcm-add-NPCM-reset-driver-support/20191030-101136
base:   https://git.pengutronix.de/git/pza/linux reset/next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sparc64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers//reset/reset-npcm.c: In function 'npcm_rc_probe':
>> drivers//reset/reset-npcm.c:147:2: warning: 'gcr_regmap' may be used uninitialized in this function [-Wmaybe-uninitialized]
     regmap_read(gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers//reset/reset-npcm.c:131:17: note: 'gcr_regmap' was declared here
     struct regmap *gcr_regmap;
                    ^~~~~~~~~~

vim +/gcr_regmap +147 drivers//reset/reset-npcm.c

   122	
   123	/*
   124	 *  The following procedure should be observed in USB PHY, USB device and
   125	 *  USB host initialization at BMC boot
   126	 */
   127	static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
   128	{
   129		struct device_node *np = pdev->dev.of_node;
   130		u32 mdlr, iprst1, iprst2, iprst3;
   131		struct regmap *gcr_regmap;
   132		u32 ipsrst1_bits = 0;
   133		u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
   134		u32 ipsrst3_bits = 0;
   135	
   136		if (of_device_is_compatible(np, "nuvoton,npcm750-reset")) {
   137			gcr_regmap = syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
   138			if (IS_ERR(gcr_regmap)) {
   139				dev_err(&pdev->dev, "Failed to find nuvoton,npcm750-gcr\n");
   140				return PTR_ERR(gcr_regmap);
   141			}
   142		}
   143		if (!gcr_regmap)
   144			return -ENXIO;
   145	
   146		/* checking which USB device is enabled */
 > 147		regmap_read(gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
   148		if (!(mdlr & NPCM_MDLR_USBD0))
   149			ipsrst3_bits |= NPCM_IPSRST3_USBD0;
   150		if (!(mdlr & NPCM_MDLR_USBD1))
   151			ipsrst1_bits |= NPCM_IPSRST1_USBD1;
   152		if (!(mdlr & NPCM_MDLR_USBD2_4))
   153			ipsrst1_bits |= (NPCM_IPSRST1_USBD2 |
   154					 NPCM_IPSRST1_USBD3 |
   155					 NPCM_IPSRST1_USBD4);
   156		if (!(mdlr & NPCM_MDLR_USBD0)) {
   157			ipsrst1_bits |= (NPCM_IPSRST1_USBD5 |
   158					 NPCM_IPSRST1_USBD6);
   159			ipsrst3_bits |= (NPCM_IPSRST3_USBD7 |
   160					 NPCM_IPSRST3_USBD8 |
   161					 NPCM_IPSRST3_USBD9);
   162		}
   163	
   164		/* assert reset USB PHY and USB devices */
   165		iprst1 = readl(rc->base + NPCM_IPSRST1);
   166		iprst2 = readl(rc->base + NPCM_IPSRST2);
   167		iprst3 = readl(rc->base + NPCM_IPSRST3);
   168	
   169		iprst1 |= ipsrst1_bits;
   170		iprst2 |= ipsrst2_bits;
   171		iprst3 |= (ipsrst3_bits | NPCM_IPSRST3_USBPHY1 |
   172			   NPCM_IPSRST3_USBPHY2);
   173	
   174		writel(iprst1, rc->base + NPCM_IPSRST1);
   175		writel(iprst2, rc->base + NPCM_IPSRST2);
   176		writel(iprst3, rc->base + NPCM_IPSRST3);
   177	
   178		/* clear USB PHY RS bit */
   179		regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
   180				   NPCM_USBXPHYCTL_RS, 0);
   181		regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
   182				   NPCM_USBXPHYCTL_RS, 0);
   183	
   184		/* deassert reset USB PHY */
   185		iprst3 &= ~(NPCM_IPSRST3_USBPHY1 | NPCM_IPSRST3_USBPHY2);
   186		writel(iprst3, rc->base + NPCM_IPSRST3);
   187	
   188		udelay(50);
   189	
   190		/* set USB PHY RS bit */
   191		regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
   192				   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
   193		regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
   194				   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
   195	
   196		/* deassert reset USB devices*/
   197		iprst1 &= ~ipsrst1_bits;
   198		iprst2 &= ~ipsrst2_bits;
   199		iprst3 &= ~ipsrst3_bits;
   200	
   201		writel(iprst1, rc->base + NPCM_IPSRST1);
   202		writel(iprst2, rc->base + NPCM_IPSRST2);
   203		writel(iprst3, rc->base + NPCM_IPSRST3);
   204	
   205		return 0;
   206	}
   207	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59112 bytes --]

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

end of thread, other threads:[~2019-10-30  5:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28 15:54 [PATCH v2 0/3] reset: npcm: add NPCM reset driver support Tomer Maimon
2019-10-28 15:54 ` [PATCH v2 1/3] dt-binding: reset: add NPCM reset controller documentation Tomer Maimon
2019-10-29 15:14   ` Philipp Zabel
2019-10-28 15:54 ` [PATCH v2 2/3] dt-bindings: reset: Add binding constants for NPCM7xx reset controller Tomer Maimon
2019-10-29 15:15   ` Philipp Zabel
2019-10-28 15:54 ` [PATCH v2 3/3] reset: npcm: add NPCM reset controller driver Tomer Maimon
2019-10-29 15:34   ` Philipp Zabel
2019-10-30  5:11   ` kbuild test robot

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).