[PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Timur Tabi]

The ARM Server Base System Architecture is a specification for ARM-based
server systems.  Among other things, it defines the behavior and register
interface for a watchdog timer.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---

[v2]
added PM support
now depends on Fu Wei's "ACPI: import watchdog info of GTDT into platform device"
 patch (GTDT platform code removed)
driver compiles as a module
replaced ARM-specific timer references
hardware registers now marked as little-endian
add OF support

 drivers/watchdog/Kconfig        |   9 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/arm_sbsa_wdt.c | 303 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 313 insertions(+)
 create mode 100644 drivers/watchdog/arm_sbsa_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e5e7c55..b97919f 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -514,6 +514,15 @@ config MEDIATEK_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called mtk_wdt.
 
+config ARM_SBSA_WDT
+	tristate "ARM Server Base System Architecture watchdog"
+	depends on ARM64 || COMPILE_TEST
+	depends on ARCH_TIMER
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include watchdog timer support for ARM Server Base
+	  System Architecture (SBSA) systems.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5c19294..063ab8c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
 obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
 obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o
 obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
+obj-$(CONFIG_ARM_SBSA_WDT) += arm_sbsa_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/arm_sbsa_wdt.c b/drivers/watchdog/arm_sbsa_wdt.c
new file mode 100644
index 0000000..5af35bf
--- /dev/null
+++ b/drivers/watchdog/arm_sbsa_wdt.c
@@ -0,0 +1,303 @@
+/*
+ * Watchdog driver for SBSA-compliant watchdog timers
+ *
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * ARM Server Base System Architecture watchdog driver.
+ *
+ * Register descriptions are taken from the ARM Server Base System
+ * Architecture document (ARM-DEN-0029)
+ */
+
+#define pr_fmt(fmt) "sbsa-gwdt: " fmt
+
+#include <linux/module.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/device.h>
+#include <linux/io.h>
+
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/reboot.h>
+
+#include <asm/arch_timer.h>
+#include <linux/acpi.h>
+
+/* Watchdog Interface Identification Registers */
+struct arm_sbsa_watchdog_ident {
+	__le32 w_iidr;	/* Watchdog Interface Identification Register */
+	uint8_t res2[0xFE8 - 0xFD0];
+	__le32 w_pidr2;	/* Peripheral ID2 Register */
+};
+
+/* Watchdog Refresh Frame */
+struct arm_sbsa_watchdog_refresh {
+	__le32 wrr;		/* Watchdog Refresh Register */
+	uint8_t res1[0xFCC - 0x004];
+	struct arm_sbsa_watchdog_ident ident;
+};
+
+/* Watchdog Control Frame */
+struct arm_sbsa_watchdog_control {
+	__le32 wcs;
+	__le32 res1;
+	__le32 wor;
+	__le32 res2;
+	__le64 wcv;
+	uint8_t res3[0xFCC - 0x018];
+	struct arm_sbsa_watchdog_ident ident;
+};
+
+struct arm_sbsa_watchdog_data {
+	struct watchdog_device wdev;
+	unsigned int pm_status;
+	struct arm_sbsa_watchdog_refresh __iomem *refresh;
+	struct arm_sbsa_watchdog_control __iomem *control;
+};
+
+static int arm_sbsa_wdt_start(struct watchdog_device *wdev)
+{
+	struct arm_sbsa_watchdog_data *data =
+		container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
+
+	/* Writing to the control register will also reset the counter */
+	writel(cpu_to_le32(1), &data->control->wcs);
+
+	return 0;
+}
+
+static int arm_sbsa_wdt_stop(struct watchdog_device *wdev)
+{
+	struct arm_sbsa_watchdog_data *data =
+		container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
+
+	writel(cpu_to_le32(0), &data->control->wcs);
+
+	return 0;
+}
+
+static int arm_sbsa_wdt_set_timeout(struct watchdog_device *wdev,
+	unsigned int timeout)
+{
+	struct arm_sbsa_watchdog_data *data =
+		container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
+	uint32_t wor;
+
+	wdev->timeout = timeout;
+
+	wor = arch_timer_get_cntfrq() * wdev->timeout;
+	writel(cpu_to_le32(wor), &data->control->wor);
+
+	return 0;
+}
+
+static int arm_sbsa_wdt_ping(struct watchdog_device *wdev)
+{
+	struct arm_sbsa_watchdog_data *data =
+		container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
+
+	writel(cpu_to_le32(1), &data->refresh->wrr);
+
+	return 0;
+}
+
+static unsigned int arm_sbsa_wdt_status(struct watchdog_device *wdev)
+{
+	struct arm_sbsa_watchdog_data *data =
+		container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
+
+	return le32_to_cpu(readl(&data->control->wcs) & 1) << WDOG_ACTIVE;
+}
+
+static unsigned int arm_sbsa_wdt_timeleft(struct watchdog_device *wdev)
+{
+	struct arm_sbsa_watchdog_data *data =
+		container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
+	uint64_t wcv = le64_to_cpu(readq(&data->control->wcv));
+	uint64_t diff = wcv - arch_counter_get_cntvct();
+
+	return diff / arch_timer_get_cntfrq();
+}
+
+static irqreturn_t arm_sbsa_wdt_interrupt(int irq, void *p)
+{
+	/*
+	 * The WS0 interrupt occurs after the first timeout, so we attempt
+	 * a manual reboot.  If this doesn't work, the WS1 timeout will
+	 * cause a hardware reset.
+	 */
+	pr_crit("Initiating system reboot\n");
+	emergency_restart();
+
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * Disable watchdog if it is active during suspend
+ */
+static int arm_sbsa_wdt_suspend(struct device *dev)
+{
+	struct arm_sbsa_watchdog_data *data = dev_get_drvdata(dev);
+
+	data->pm_status = le32_to_cpu(readl(&data->control->wcs)) & 1;
+
+	if (data->pm_status)
+		writel(cpu_to_le32(0), &data->control->wcs);
+
+	return 0;
+}
+
+/*
+ * Enable watchdog and configure it if necessary
+ */
+static int arm_sbsa_wdt_resume(struct device *dev)
+{
+	struct arm_sbsa_watchdog_data *data = dev_get_drvdata(dev);
+
+	if (data->pm_status)
+		writel(cpu_to_le32(0), &data->control->wcs);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops arm_sbsa_wdt_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(arm_sbsa_wdt_suspend, arm_sbsa_wdt_resume)
+};
+
+static struct watchdog_info arm_sbsa_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity = "ARM SBSA watchdog",
+};
+
+static struct watchdog_ops arm_sbsa_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = arm_sbsa_wdt_start,
+	.stop = arm_sbsa_wdt_stop,
+	.ping = arm_sbsa_wdt_ping,
+	.set_timeout = arm_sbsa_wdt_set_timeout,
+	.status = arm_sbsa_wdt_status,
+	.get_timeleft = arm_sbsa_wdt_timeleft,
+};
+
+static int __init arm_sbsa_wdt_probe(struct platform_device *pdev)
+{
+	struct arm_sbsa_watchdog_data *data;
+	struct resource *res;
+	uint32_t iidr;
+	int irq, ret;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
+	data->control = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->control))
+		return PTR_ERR(data->control);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "refresh");
+	data->refresh = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->refresh))
+		return PTR_ERR(data->refresh);
+
+	iidr = le32_to_cpu(readl(&data->control->ident.w_iidr));
+
+	/* We only support architecture version 0 */
+	if (((iidr >> 16) & 0xf) != 0) {
+		dev_info(&pdev->dev,
+			 "only architecture version 0 is supported\n");
+		return -ENODEV;
+	}
+
+	irq = platform_get_irq_byname(pdev, "ws0");
+	if (irq < 0) {
+		dev_err(&pdev->dev, "could not get interrupt\n");
+		return irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, arm_sbsa_wdt_interrupt,
+			       0, pdev->name, NULL);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "could not request irq %i (ret=%i)\n",
+			irq, ret);
+		return ret;
+	}
+
+	data->wdev.info = &arm_sbsa_wdt_info;
+	data->wdev.ops = &arm_sbsa_wdt_ops;
+	data->wdev.min_timeout = 1;
+	data->wdev.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
+
+	/* Calculate the maximum timeout in seconds that we can support */
+	data->wdev.max_timeout = U32_MAX / arch_timer_get_cntfrq();
+
+	watchdog_set_drvdata(&data->wdev, data);
+
+	/*
+	 * Bits [15:12] are an implementation-defined revision number
+	 * for the component.
+	 */
+	arm_sbsa_wdt_info.firmware_version = (iidr >> 12) & 0xf;
+
+	ret = watchdog_register_device(&data->wdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"could not register watchdog device (ret=%i)\n", ret);
+		return ret;
+	}
+
+	dev_dbg(&pdev->dev, "implementer code is %03x\n",
+		 (iidr & 0xf00) >> 1 | (iidr & 0x7f));
+	dev_info(&pdev->dev, "maximum timeout is %u seconds\n",
+		 data->wdev.max_timeout);
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+}
+
+static int __exit arm_sbsa_wdt_remove(struct platform_device *pdev)
+{
+	struct arm_sbsa_watchdog_data *data = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&data->wdev);
+
+	return 0;
+}
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id arm_sbsa_wdt_of_match[] = {
+	{ .compatible = "arm,sbsa-gwdt" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, arm_sbsa_wdt_of_match);
+#endif
+
+static struct platform_driver arm_sbsa_wdt_driver = {
+	.driver = {
+		.name = "sbsa-gwdt",
+		.owner = THIS_MODULE,
+		.pm = &arm_sbsa_wdt_pm_ops,
+		.of_match_table = of_match_ptr(arm_sbsa_wdt_of_match),
+	},
+	.probe = arm_sbsa_wdt_probe,
+	.remove = __exit_p(arm_sbsa_wdt_remove),
+};
+
+module_platform_driver(arm_sbsa_wdt_driver);
+
+MODULE_DESCRIPTION("ARM Server Base System Architecture Watchdog Driver");
+MODULE_LICENSE("GPL v2");
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Arnd Bergmann]

On Thursday 14 May 2015 11:52:33 Timur Tabi wrote:
> +static int arm_sbsa_wdt_set_timeout(struct watchdog_device *wdev,
> +       unsigned int timeout)
> +{
> +       struct arm_sbsa_watchdog_data *data =
> +               container_of(wdev, struct arm_sbsa_watchdog_data, wdev);
> +       uint32_t wor;
> +
> +       wdev->timeout = timeout;
> +
> +       wor = arch_timer_get_cntfrq() * wdev->timeout;
> +       writel(cpu_to_le32(wor), &data->control->wor);

Why the double endianess swap? Are the registers defined to be CPU-endian?

You probably mean le32_to_cpu() here, not the other way round, although 
the effect is the same.

	Arnd

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Timur Tabi]

On 05/14/2015 03:03 PM, Arnd Bergmann wrote:
>> >+       writel(cpu_to_le32(wor), &data->control->wor);
> Why the double endianess swap? Are the registers defined to be CPU-endian?
>
> You probably mean le32_to_cpu() here, not the other way round, although
> the effect is the same.

Maybe I'd doing the whole __le32 thing wrong, but I think 
cpu_to_le32(wor) is correct.  'wor' is a normal integer, so it's in 
"cpu" format.  I want to convert it to le32 format so that I write an 
le32 value to memory.  Therefore, cpu_to_le32().  Most calls to writel() 
use cpu_to_le32():

$ git grep -l "writel(cpu_to_le32"
arch/m68k/coldfire/pci.c
arch/mips/pci/ops-bcm63xx.c
arch/mips/pci/ops-bonito64.c
arch/mips/pci/ops-loongson2.c
arch/parisc/include/asm/io.h
drivers/atm/fore200e.c
drivers/block/umem.c
drivers/isdn/hardware/mISDN/hfcmulti.c
drivers/mtd/nand/r852.c
drivers/net/ethernet/brocade/bna/bfa_ioc.c
drivers/parisc/eisa.c
drivers/scsi/bfa/bfa_ioc.c
drivers/scsi/bnx2fc/bnx2fc_hwi.c
drivers/scsi/bnx2i/bnx2i_hwi.c
drivers/scsi/hptiop.c
drivers/scsi/mpt2sas/mpt2sas_base.c
drivers/scsi/mpt3sas/mpt3sas_base.c
drivers/scsi/nsp32_io.h
drivers/staging/i2o/i2o.h
drivers/usb/isp1760/isp1760-hcd.c
sound/pci/mixart/mixart_hwdep.h

$ git grep -l "writel(le32_to_cpu"
drivers/scsi/hpsa.c
drivers/scsi/megaraid/megaraid_sas_fusion.c
drivers/virtio/virtio_mmio.c



-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Guenter Roeck]

On Thu, May 14, 2015 at 03:15:55PM -0500, Timur Tabi wrote:
> On 05/14/2015 03:03 PM, Arnd Bergmann wrote:
> >>>+       writel(cpu_to_le32(wor), &data->control->wor);
> >Why the double endianess swap? Are the registers defined to be CPU-endian?
> >
> >You probably mean le32_to_cpu() here, not the other way round, although
> >the effect is the same.
> 
> Maybe I'd doing the whole __le32 thing wrong, but I think cpu_to_le32(wor)
> is correct.  'wor' is a normal integer, so it's in "cpu" format.  I want to
> convert it to le32 format so that I write an le32 value to memory.
> Therefore, cpu_to_le32().  Most calls to writel() use cpu_to_le32():
> 
What other drivers do is completely irrelevant. The key question is what
_this_ hardware does, and you should know that.

It concerns me that you point to other drivers as argument for using the
endianness conversion your code. That raises the question if you really
understand the hardware. Is the register known to be a little endian register,
even on a big endian machine, or is it in host byte order ? You don't really
answer that question.

Guenter

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Timur Tabi]

On 05/14/2015 03:45 PM, Guenter Roeck wrote:
> It concerns me that you point to other drivers as argument for using the
> endianness conversion your code. That raises the question if you really
> understand the hardware.

The only information I have on how the hardware works is the SBSA 
document and the internal documentation for our ARM64 server chip.

 > Is the register known to be a little endian register,
> even on a big endian machine, or is it in host byte order ? You don't really
> answer that question.

The ARM SBSA says in section 4.1.1:

	* CPUs shall implement little-endian support.

That's the only reference to endianness in the document.  I don't know 
if that means that the device registers must also be little-endian. 
Does Linux even support running ARM64 in big-endian mode?

So maybe it's safe to say that everything must be little endian?  If so, 
do I still need the __le32 in the structs, and should I still use 
cpu_to_le32()?

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Guenter Roeck]

On 05/14/2015 02:03 PM, Timur Tabi wrote:
> On 05/14/2015 03:45 PM, Guenter Roeck wrote:
>> It concerns me that you point to other drivers as argument for using the
>> endianness conversion your code. That raises the question if you really
>> understand the hardware.
>
> The only information I have on how the hardware works is the SBSA document and the internal documentation for our ARM64 server chip.
>
>  > Is the register known to be a little endian register,
>> even on a big endian machine, or is it in host byte order ? You don't really
>> answer that question.
>
> The ARM SBSA says in section 4.1.1:
>
>      * CPUs shall implement little-endian support.
>
That doesn't really say anything. It neither says that the CPU must _run_
in little endian mode, nor does it say anything about its register byte order.

> That's the only reference to endianness in the document.  I don't know if that means that the device registers must also be little-endian. Does Linux even support running ARM64 in big-endian mode?
>

Interesting question. You should be able to answer it yourself.
Hint: Look in arch/arm64/Kconfig.

> So maybe it's safe to say that everything must be little endian?  If so, do I still need the __le32 in the structs, and should I still use cpu_to_le32()?
>

We still don't know if the registers are in host byte order or in little endian
order. If registers are in host byte order, there is no need for a conversion.

Guenter

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Timur Tabi]

Guenter Roeck wrote:
>
> We still don't know if the registers are in host byte order or in little
> endian
> order. If registers are in host byte order, there is no need for a
> conversion.

Well, if I compile and boot a big-endian ARM64 kernel, then I don't 
expect the hardware devices to magically switch all their registers into 
big endian.  The devices will remain little-endian.

I presume that this is true on all ARM and ARM64 systems.  Switching the 
CPU into big-endian mode (when it normally would run in little-endian) 
does not also switch the hardware registers.

Therefore, if we want to support big-endian kernels on ARM64, then every 
readl/writel in every driver must use cpu_to_le32/etc.

I think this conversation has gone off-track.  I don't see how the SBSA 
watchdog device is any different from any other device on an ARM64 platform.

I think it's safe to say that the endian order is not specified 
anywhere, but on my hardware, everything is little-endian.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Arnd Bergmann]

On Thursday 14 May 2015 18:49:17 Timur Tabi wrote:
> Guenter Roeck wrote:
> >
> > We still don't know if the registers are in host byte order or in little
> > endian
> > order. If registers are in host byte order, there is no need for a
> > conversion.
> 
> Well, if I compile and boot a big-endian ARM64 kernel, then I don't 
> expect the hardware devices to magically switch all their registers into 
> big endian.  The devices will remain little-endian.
> 
> I presume that this is true on all ARM and ARM64 systems.  Switching the 
> CPU into big-endian mode (when it normally would run in little-endian) 
> does not also switch the hardware registers.
> 
> Therefore, if we want to support big-endian kernels on ARM64, then every 
> readl/writel in every driver must use cpu_to_le32/etc.
> 
> I think this conversation has gone off-track.  I don't see how the SBSA 
> watchdog device is any different from any other device on an ARM64 platform.
> 
> I think it's safe to say that the endian order is not specified 
> anywhere, but on my hardware, everything is little-endian.

Then please remove the double-swap. If the device works like any other
device in the system, then the byteswap that is implied by readl/writel
will do the right thing, and swapping twice will break big-endian
kernels.

	Arnd

Re: [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Timur Tabi]

Arnd Bergmann wrote:
> Then please remove the double-swap. If the device works like any other
> device in the system, then the byteswap that is implied by readl/writel
> will do the right thing, and swapping twice will break big-endian
> kernels.

Should I still keep the __le32 in the structs?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [Linaro-acpi] [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Arnd Bergmann]

On Friday 15 May 2015 07:14:08 Timur Tabi wrote:
> Arnd Bergmann wrote:
> > Then please remove the double-swap. If the device works like any other
> > device in the system, then the byteswap that is implied by readl/writel
> > will do the right thing, and swapping twice will break big-endian
> > kernels.
> 
> Should I still keep the __le32 in the structs?

Not sure about that. Most people use offsets with a void * pointer,
so they would not care.

Do you get a sparse warning if you either leave the __le32, or if you
turn it into u32? The correct behavior is probably to leave __le32
here, and ensure that readl() produces a sparse warning when fed with
a pointer to u32, but not void* or __le32*. It would need some
experimenting to find if that catches real bugs or just starts warning
about correct code and not about incorrect code.

FWIW, the drivers you cited that use le32_to_cpu(readl(p)) are probably
all wrong, and it would be nice to have a sparse warning for that.

	Arnd

Re: [Linaro-acpi] [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Timur Tabi]

On 05/15/2015 08:20 AM, Arnd Bergmann wrote:

> Do you get a sparse warning if you either leave the __le32, or if you
> turn it into u32?

After removing the cpu_to_le32 calls, but keeping the __le32 in the 
structs, I get no sparse warnings on my driver.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [Linaro-acpi] [PATCH] [v2] watchdog: introduce the ARM64 SBSA watchdog driver

[Arnd Bergmann]

On Friday 15 May 2015 11:21:05 Timur Tabi wrote:
> On 05/15/2015 08:20 AM, Arnd Bergmann wrote:
> 
> > Do you get a sparse warning if you either leave the __le32, or if you
> > turn it into u32?
> 
> After removing the cpu_to_le32 calls, but keeping the __le32 in the 
> structs, I get no sparse warnings on my driver.

Ok, good. I've also checked the readl() definition and found that it
has a (__force __le32) cast, while __raw_readl() takes a const volatile
void __iomem *addr) pointer, so it sparse won't warn either way, but
that's probably ok.


	Arnd