* [PATCH v5 4/7] power/restart: Call machine_restart instead of arm_pm_restart
From: Guenter Roeck @ 2014-07-18 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405668856-13738-1-git-send-email-linux@roeck-us.net>
machine_restart is supported on non-ARM platforms, and and ultimately calls
arm_pm_restart, so dont call arm_pm_restart directly but use the more
generic function.
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change.
v4: No change.
v3: No change.
v2: Added patch.
drivers/power/reset/restart-poweroff.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/power/reset/restart-poweroff.c b/drivers/power/reset/restart-poweroff.c
index 5758033..47f10eb 100644
--- a/drivers/power/reset/restart-poweroff.c
+++ b/drivers/power/reset/restart-poweroff.c
@@ -20,7 +20,8 @@
static void restart_poweroff_do_poweroff(void)
{
- arm_pm_restart(REBOOT_HARD, NULL);
+ reboot_mode = REBOOT_HARD;
+ machine_restart(NULL);
}
static int restart_poweroff_probe(struct platform_device *pdev)
--
1.9.1
^ permalink raw reply related
* [PATCH v5 3/7] arm: Support restart through restart handler call chain
From: Guenter Roeck @ 2014-07-18 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405668856-13738-1-git-send-email-linux@roeck-us.net>
The kernel core now supports a restart handler call chain for system
restart functions.
With this change, the arm_pm_restart callback is now optional, so check
if it is set before calling it. Only call the kernel restart handler
if arm_pm_restart is not set.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Renamed restart function to do_kernel_restart
v4: No change.
v3: Use wrapper function to execute notifier call chain.
v2: Only call notifier call chain if arm_pm_restart is not set.
Do not include linux/watchdog.h.
arch/arm/kernel/process.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..84ca0d5 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -230,7 +230,10 @@ void machine_restart(char *cmd)
local_irq_disable();
smp_send_stop();
- arm_pm_restart(reboot_mode, cmd);
+ if (arm_pm_restart)
+ arm_pm_restart(reboot_mode, cmd);
+ else
+ do_kernel_restart(cmd);
/* Give a grace period for failure to restart of 1s */
mdelay(1000);
--
1.9.1
^ permalink raw reply related
* [PATCH v5 2/7] arm64: Support restart through restart handler call chain
From: Guenter Roeck @ 2014-07-18 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405668856-13738-1-git-send-email-linux@roeck-us.net>
The kernel core now supports a restart handler call chain to restart
the system. Call it if arm_pm_restart is not set.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Renamed restart function to do_kernel_restart
v4: No change.
v3: Use wrapper function to execute notifier call chain.
v2: Only call notifier call chain if arm_pm_restart is not set.
Do not include linux/watchdog.h.
arch/arm64/kernel/process.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 43b7c34..9591e60 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -174,6 +174,8 @@ void machine_restart(char *cmd)
/* Now call the architecture specific reboot code. */
if (arm_pm_restart)
arm_pm_restart(reboot_mode, cmd);
+ else
+ do_kernel_restart(cmd);
/*
* Whoops - the architecture was unable to reboot.
--
1.9.1
^ permalink raw reply related
* [PATCH v5 1/7] kernel: Add support for kernel restart handler call chain
From: Guenter Roeck @ 2014-07-18 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405668856-13738-1-git-send-email-linux@roeck-us.net>
Various drivers implement architecture and/or device specific means
to restart (reset) the system. Various mechanisms have been implemented
to support those schemes. The best known mechanism is arm_pm_restart,
which is a function pointer to be set either from platform specific code
or from drivers. Another mechanism is to use hardware watchdogs to issue
a reset; this mechanism is used if there is no other method available
to reset a board or system. Two examples are alim7101_wdt, which currently
uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
the arm_pm_restart function.
The existing mechanisms have a number of drawbacks. Typically only one scheme
to restart the system is supported (at least if arm_pm_restart is used).
At least in theory there can be multiple means to restart the system, some of
which may be less desirable (for example one mechanism may only reset the CPU,
while another may reset the entire system). Using arm_pm_restart can also be
racy if the function pointer is set from a driver, as the driver may be in
the process of being unloaded when arm_pm_restart is called.
Using the reboot notifier is always racy, as it is unknown if and when
other functions using the reboot notifier have completed execution
by the time the watchdog fires.
Introduce a system restart handler call chain to solve the described problems.
This call chain is expected to be executed from the architecture specific
machine_restart() function. Drivers providing system restart functionality
(such as the watchdog drivers mentioned above) are expected to register
with this call chain. By using the priority field in the notifier block,
callers can control restart handler execution sequence and thus ensure that
the restart handler with the optimal restart capabilities for a given system
is called first.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Function renames:
register_restart_notifier -> register_restart_handler
unregister_restart_notifier -> unregister_restart_handler
kernel_restart_notify -> do_kernel_restart
v4: Document and suggest values for notifier priorities
v3: Add kernel_restart_notify wrapper function to execute notifier.
Improve documentation.
Move restart_notifier_list into kernel/reboot.c and make it static.
v2: No change.
include/linux/reboot.h | 3 ++
kernel/reboot.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+)
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 48bf152..67fc8fc 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -38,6 +38,9 @@ extern int reboot_force;
extern int register_reboot_notifier(struct notifier_block *);
extern int unregister_reboot_notifier(struct notifier_block *);
+extern int register_restart_handler(struct notifier_block *);
+extern int unregister_restart_handler(struct notifier_block *);
+extern void do_kernel_restart(char *cmd);
/*
* Architecture-specific implementations of sys_reboot commands.
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a3a9e24..33e8170 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -104,6 +104,87 @@ int unregister_reboot_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_reboot_notifier);
+/*
+ * Notifier list for kernel code which wants to be called
+ * to restart the system.
+ */
+static BLOCKING_NOTIFIER_HEAD(restart_handler_list);
+
+/**
+ * register_restart_handler - Register function to be called to reset
+ * the system
+ * @nb: Info about handler function to be called
+ * @nb->priority: Handler priority. Handlers should follow the
+ * following guidelines for setting priorities.
+ * 0: Restart handler of last resort,
+ * with limited restart capabilities
+ * 128: Default restart handler; use if no other
+ * restart handler is expected to be available,
+ * and/or if restart functionality is
+ * sufficient to restart the entire system
+ * 255: Highest priority restart handler, will
+ * preempt all other restart handlers
+ *
+ * Registers a function with the list of functions
+ * to be called to restart the system.
+ *
+ * Registered functions will be called from machine_restart as last
+ * step of the restart sequence (if the architecture specific
+ * machine_restart function calls do_kernel_restart - see below
+ * for details).
+ * Registered functions are expected to restart the system immediately.
+ * If more than one function is registered, the restart handler priority
+ * selects which function will be called first.
+ *
+ * Restart handlers are expected to be registered from non-architecture
+ * code, typically from drivers. A typical use case would be a system
+ * where restart functionality is provided through a watchdog. Multiple
+ * restart handlers may exist; for example, one restart handler might
+ * restart the entire system, while another only restarts the CPU.
+ * In such cases, the restart handler which only restarts part of the
+ * hardware is expected to register with low priority to ensure that
+ * it only runs if no other means to restart the system is available.
+ *
+ * Currently always returns zero, as blocking_notifier_chain_register()
+ * always returns zero.
+ */
+int register_restart_handler(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&restart_handler_list, nb);
+}
+EXPORT_SYMBOL(register_restart_handler);
+
+/**
+ * unregister_restart_handler - Unregister previously registered
+ * restart handler
+ * @nb: Hook to be unregistered
+ *
+ * Unregisters a previously registered restart handler function.
+ *
+ * Returns zero on success, or %-ENOENT on failure.
+ */
+int unregister_restart_handler(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&restart_handler_list, nb);
+}
+EXPORT_SYMBOL(unregister_restart_handler);
+
+/**
+ * do_kernel_restart - Execute kernel restart handler call chain
+ *
+ * Calls functions registered with register_restart_handler.
+ *
+ * Expected to be called from machine_restart as last step of the restart
+ * sequence.
+ *
+ * Restarts the system immediately if a restart handler function has been
+ * registered. Otherwise does nothing.
+ */
+void do_kernel_restart(char *cmd)
+{
+ blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
+}
+
void migrate_to_reboot_cpu(void)
{
/* The boot cpu is always logical cpu 0 */
--
1.9.1
^ permalink raw reply related
* [PATCH v5 0/7] kernel: Add support for restart handler call chain
From: Guenter Roeck @ 2014-07-18 7:34 UTC (permalink / raw)
To: linux-arm-kernel
Various drivers implement architecture and/or device specific means
to restart (reset) the system. Various mechanisms have been implemented
to support those schemes. The best known mechanism is arm_pm_restart,
which is a function pointer to be set either from platform specific code
or from drivers. Another mechanism is to use hardware watchdogs to issue
a reset; this mechanism is used if there is no other method available
to reset a board or system. Two examples are alim7101_wdt, which currently
uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
the arm_pm_restart function. Several other restart drivers for arm, all
directly calling arm_pm_restart, are in the process of being integrated
into the kernel. All those drivers would benefit from the new API.
The existing mechanisms have a number of drawbacks. Typically only one scheme
to restart the system is supported (at least if arm_pm_restart is used).
At least in theory there can be multiple means to restart the system, some of
which may be less desirable (for example one mechanism may only reset the CPU,
while another may reset the entire system). Using arm_pm_restart can also be
racy if the function pointer is set from a driver, as the driver may be in
the process of being unloaded when arm_pm_restart is called.
Using the reboot notifier is always racy, as it is unknown if and when
other functions using the reboot notifier have completed execution
by the time the watchdog fires.
Introduce a system restart handler call chain to solve the described problems.
This call chain is expected to be executed from the architecture specific
machine_restart() function. Drivers providing system restart functionality
(such as the watchdog drivers mentioned above) are expected to register
with this call chain. By using the priority field in the notifier block,
callers can control restart handler execution sequence and thus ensure that
the restart handler with the optimal restart capabilities for a given system
is called first.
Since the first revision of this patchset, a number of separate patch
submissions have been made which either depend on it or could make use of it.
http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
registers three notifiers.
https://lkml.org/lkml/2014/7/8/962
would benefit from it.
Patch 1 of this series implements the restart handler function. Patches 2 and 3
implement calling the restart handler chain from arm and arm64 restart code.
Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
directly but machine_restart. This is done to avoid calling arm_pm_restart
from more than one place. The change makes the driver architecture independent,
so it would be possible to drop the arm dependency from its Kconfig entry.
Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
that no one gets the idea to implement a restart handler as module.
---
v5: Rebased series to v3.16-rc5
Function renames:
register_restart_notifier -> register_restart_handler
unregister_restart_notifier -> unregister_restart_handler
kernel_restart_notify -> do_kernel_restart
v4: Document restart notifier priorities
Select 128 as default priority for newly introduced notifiers
Fix checkpatch warning (line too long) in moxart patch
v3: Drop RFC.
Add kernel_restart_notify wrapper function to execute notifier
Improve documentation.
Move restart_notifier_list into kernel/reboot.c and make it static.
v2: Add patch 4.
Only call blocking notifier call chain if arm_pm_restart was not set.
^ permalink raw reply
* [PATCH] CMA: generalize CMA reserved area management functionality (fixup)
From: Marek Szyprowski @ 2014-07-18 7:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140717150615.32c48786b6bdbc880bdc5ed4@linux-foundation.org>
Hello,
On 2014-07-18 00:06, Andrew Morton wrote:
> On Thu, 17 Jul 2014 11:36:07 +0200 Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
>> MAX_CMA_AREAS is used by other subsystems (i.e. arch/arm/mm/dma-mapping.c),
>> so we need to provide correct definition even if CMA is disabled.
>> This patch fixes this issue.
>>
>> Reported-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> include/linux/cma.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/cma.h b/include/linux/cma.h
>> index 9a18a2b1934c..c077635cad76 100644
>> --- a/include/linux/cma.h
>> +++ b/include/linux/cma.h
>> @@ -5,7 +5,11 @@
>> * There is always at least global CMA area and a few optional
>> * areas configured in kernel .config.
>> */
>> +#ifdef CONFIG_CMA
>> #define MAX_CMA_AREAS (1 + CONFIG_CMA_AREAS)
>> +#else
>> +#define MAX_CMA_AREAS (0)
>> +#endif
>>
>> struct cma;
> Joonsoo already fixed this up, a bit differently:
> http://ozlabs.org/~akpm/mmots/broken-out/cma-generalize-cma-reserved-area-management-functionality-fix.patch
>
> Which approach makes more sense?
CMA_AREAS depends on CMA being enabled, so both approaches works exactly
the same way. Please keep Joonsoo's patch and just ignore mine to avoid
confusing others by disappearing patches. I'm sorry that I've missed it
before sending mine.
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Subject: CMA: fix ARM build failure related to MAX_CMA_AREAS definition
>
> If CMA is disabled, CONFIG_CMA_AREAS isn't defined so compile error
> happens. To fix it, define MAX_CMA_AREAS if CONFIG_CMA_AREAS
> isn't defined.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> include/linux/cma.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff -puN include/linux/cma.h~cma-generalize-cma-reserved-area-management-functionality-fix include/linux/cma.h
> --- a/include/linux/cma.h~cma-generalize-cma-reserved-area-management-functionality-fix
> +++ a/include/linux/cma.h
> @@ -5,8 +5,14 @@
> * There is always at least global CMA area and a few optional
> * areas configured in kernel .config.
> */
> +#ifdef CONFIG_CMA_AREAS
> #define MAX_CMA_AREAS (1 + CONFIG_CMA_AREAS)
>
> +#else
> +#define MAX_CMA_AREAS (0)
> +
> +#endif
> +
> struct cma;
>
> extern phys_addr_t cma_get_base(struct cma *cma);
> _
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH v5 1/8] of: Add NVIDIA Tegra SATA controller binding
From: Mikko Perttunen @ 2014-07-18 7:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405667500-7402-1-git-send-email-mperttunen@nvidia.com>
So here's v5: this time, as suggested, I handle the sata clock myself
and let ahci_platform handle it too, leading it to be prepared+enabled
twice. This works fine, and allows us to remove the DT ordering requirement.
I also have in the works a patchset that adds the name-based
ahci_platform_get_resources function, but that is not quite ready yet,
even if it is quite far along. Also, I am going on vacation and
returning on 28.7., so if this v5 is acceptable maybe it could be merged
for 3.17 and I could work on the new get_resources scheme after I get
back from vacation?
Thanks, Mikko
On 18/07/14 10:11, Mikko Perttunen wrote:
> This patch adds device tree binding documentation for the SATA
> controller found on NVIDIA Tegra SoCs.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> v5: remove ordering requirement again
>
> .../devicetree/bindings/ata/tegra-sata.txt | 30 ++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/ata/tegra-sata.txt
>
> diff --git a/Documentation/devicetree/bindings/ata/tegra-sata.txt b/Documentation/devicetree/bindings/ata/tegra-sata.txt
> new file mode 100644
> index 0000000..946f207
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/tegra-sata.txt
> @@ -0,0 +1,30 @@
> +Tegra124 SoC SATA AHCI controller
> +
> +Required properties :
> +- compatible : "nvidia,tegra124-ahci".
> +- reg : Should contain 2 entries:
> + - AHCI register set (SATA BAR5)
> + - SATA register set
> +- interrupts : Defines the interrupt used by SATA
> +- clocks : Must contain an entry for each entry in clock-names.
> + See ../clocks/clock-bindings.txt for details.
> +- clock-names : Must include the following entries:
> + - sata
> + - sata-oob
> + - cml1
> + - pll_e
> +- resets : Must contain an entry for each entry in reset-names.
> + See ../reset/reset.txt for details.
> +- reset-names : Must include the following entries:
> + - sata
> + - sata-oob
> + - sata-cold
> +- phys : Must contain an entry for each entry in phy-names.
> + See ../phy/phy-bindings.txt for details.
> +- phy-names : Must include the following entries:
> + - sata-phy : XUSB PADCTL SATA PHY
> +- hvdd-supply : Defines the SATA HVDD regulator
> +- vddio-supply : Defines the SATA VDDIO regulator
> +- avdd-supply : Defines the SATA AVDD regulator
> +- target-5v-supply : Defines the SATA 5V power regulator
> +- target-12v-supply : Defines the SATA 12V power regulator
>
^ permalink raw reply
* [PATCH v5 7/8] ata: Add support for the Tegra124 SATA controller
From: Mikko Perttunen @ 2014-07-18 7:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405500863-19696-8-git-send-email-mperttunen@nvidia.com>
This adds support for the integrated AHCI-compliant Serial ATA
controller present on the NVIDIA Tegra124 system-on-chip.
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v5: let ahci_platform handle sata clock and also handle it ourselves.
this allows use of ahci_platform while having a special sequence
for the clock.
drivers/ata/Kconfig | 9 ++
drivers/ata/Makefile | 1 +
drivers/ata/ahci_tegra.c | 377 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 387 insertions(+)
create mode 100644 drivers/ata/ahci_tegra.c
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index b0d5b5a..e1b9278 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -142,6 +142,15 @@ config AHCI_SUNXI
If unsure, say N.
+config AHCI_TEGRA
+ tristate "NVIDIA Tegra124 AHCI SATA support"
+ depends on ARCH_TEGRA
+ help
+ This option enables support for the NVIDIA Tegra124 SoC's
+ onboard AHCI SATA.
+
+ If unsure, say N.
+
config AHCI_XGENE
tristate "APM X-Gene 6.0Gbps AHCI SATA host controller support"
depends on PHY_XGENE
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 5a02aee..ae41107 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_AHCI_IMX) += ahci_imx.o libahci.o libahci_platform.o
obj-$(CONFIG_AHCI_MVEBU) += ahci_mvebu.o libahci.o libahci_platform.o
obj-$(CONFIG_AHCI_SUNXI) += ahci_sunxi.o libahci.o libahci_platform.o
obj-$(CONFIG_AHCI_ST) += ahci_st.o libahci.o libahci_platform.o
+obj-$(CONFIG_AHCI_TEGRA) += ahci_tegra.o libahci.o libahci_platform.o
obj-$(CONFIG_AHCI_XGENE) += ahci_xgene.o libahci.o libahci_platform.o
# SFF w/ custom DMA
diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
new file mode 100644
index 0000000..d30bb21
--- /dev/null
+++ b/drivers/ata/ahci_tegra.c
@@ -0,0 +1,377 @@
+/*
+ * drivers/ata/ahci_tegra.c
+ *
+ * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Author:
+ * Mikko Perttunen <mperttunen@nvidia.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/ahci_platform.h>
+#include <linux/reset.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/tegra-powergate.h>
+#include <linux/regulator/consumer.h>
+#include "ahci.h"
+
+#define SATA_CONFIGURATION_0 0x180
+#define SATA_CONFIGURATION_EN_FPCI BIT(0)
+
+#define SCFG_OFFSET 0x1000
+
+#define T_SATA0_CFG_1 0x04
+#define T_SATA0_CFG_1_IO_SPACE BIT(0)
+#define T_SATA0_CFG_1_MEMORY_SPACE BIT(1)
+#define T_SATA0_CFG_1_BUS_MASTER BIT(2)
+#define T_SATA0_CFG_1_SERR BIT(8)
+
+#define T_SATA0_CFG_9 0x24
+#define T_SATA0_CFG_9_BASE_ADDRESS_SHIFT 13
+
+#define SATA_FPCI_BAR5 0x94
+#define SATA_FPCI_BAR5_START_SHIFT 4
+
+#define SATA_INTR_MASK 0x188
+#define SATA_INTR_MASK_IP_INT_MASK BIT(16)
+
+#define T_SATA0_AHCI_HBA_CAP_BKDR 0x300
+
+#define T_SATA0_BKDOOR_CC 0x4a4
+
+#define T_SATA0_CFG_SATA 0x54c
+#define T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN BIT(12)
+
+#define T_SATA0_CFG_MISC 0x550
+
+#define T_SATA0_INDEX 0x680
+
+#define T_SATA0_CHX_PHY_CTRL1_GEN1 0x690
+#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK 0xff
+#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT 0
+#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK (0xff << 8)
+#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT 8
+
+#define T_SATA0_CHX_PHY_CTRL1_GEN2 0x694
+#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK 0xff
+#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_SHIFT 0
+#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK (0xff << 12)
+#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_SHIFT 12
+
+#define T_SATA0_CHX_PHY_CTRL2 0x69c
+#define T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1 0x23
+
+#define T_SATA0_CHX_PHY_CTRL11 0x6d0
+#define T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ (0x2800 << 16)
+
+#define FUSE_SATA_CALIB 0x124
+#define FUSE_SATA_CALIB_MASK 0x3
+
+struct sata_pad_calibration {
+ u8 gen1_tx_amp;
+ u8 gen1_tx_peak;
+ u8 gen2_tx_amp;
+ u8 gen2_tx_peak;
+};
+
+static const struct sata_pad_calibration tegra124_pad_calibration[] = {
+ {0x18, 0x04, 0x18, 0x0a},
+ {0x0e, 0x04, 0x14, 0x0a},
+ {0x0e, 0x07, 0x1a, 0x0e},
+ {0x14, 0x0e, 0x1a, 0x0e},
+};
+
+struct tegra_ahci_priv {
+ struct platform_device *pdev;
+ void __iomem *sata_regs;
+ struct reset_control *sata_rst;
+ struct reset_control *sata_oob_rst;
+ struct reset_control *sata_cold_rst;
+ /* Needs special handling, cannot use ahci_platform */
+ struct clk *sata_clk;
+ struct regulator_bulk_data supplies[5];
+};
+
+static int tegra_ahci_power_on(struct ahci_host_priv *hpriv)
+{
+ struct tegra_ahci_priv *tegra = hpriv->plat_data;
+ int ret;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
+ tegra->supplies);
+ if (ret)
+ return ret;
+
+ ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
+ tegra->sata_clk,
+ tegra->sata_rst);
+ if (ret)
+ goto disable_regulators;
+
+ reset_control_assert(tegra->sata_oob_rst);
+ reset_control_assert(tegra->sata_cold_rst);
+
+ ret = ahci_platform_enable_resources(hpriv);
+ if (ret)
+ goto disable_power;
+
+ reset_control_deassert(tegra->sata_cold_rst);
+ reset_control_deassert(tegra->sata_oob_rst);
+
+ return 0;
+
+disable_power:
+ clk_disable_unprepare(tegra->sata_clk);
+
+ tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
+
+disable_regulators:
+ regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
+
+ return ret;
+}
+
+static void tegra_ahci_power_off(struct ahci_host_priv *hpriv)
+{
+ struct tegra_ahci_priv *tegra = hpriv->plat_data;
+
+ ahci_platform_disable_resources(hpriv);
+
+ reset_control_assert(tegra->sata_rst);
+ reset_control_assert(tegra->sata_oob_rst);
+ reset_control_assert(tegra->sata_cold_rst);
+
+ clk_disable_unprepare(tegra->sata_clk);
+ tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
+
+ regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
+}
+
+static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)
+{
+ struct tegra_ahci_priv *tegra = hpriv->plat_data;
+ int ret;
+ unsigned int val;
+ struct sata_pad_calibration calib;
+
+ ret = tegra_ahci_power_on(hpriv);
+ if (ret) {
+ dev_err(&tegra->pdev->dev,
+ "failed to power on AHCI controller: %d\n", ret);
+ return ret;
+ }
+
+ val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
+ val |= SATA_CONFIGURATION_EN_FPCI;
+ writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
+
+ /* Pad calibration */
+
+ /* FIXME Always use calibration 0. Change this to read the calibration
+ * fuse once the fuse driver has landed. */
+ val = 0;
+
+ calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
+
+ writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
+
+ val = readl(tegra->sata_regs +
+ SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
+ val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
+ val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
+ val |= calib.gen1_tx_amp <<
+ T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
+ val |= calib.gen1_tx_peak <<
+ T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
+ writel(val, tegra->sata_regs + SCFG_OFFSET +
+ T_SATA0_CHX_PHY_CTRL1_GEN1);
+
+ val = readl(tegra->sata_regs +
+ SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
+ val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
+ val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
+ val |= calib.gen2_tx_amp <<
+ T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
+ val |= calib.gen2_tx_peak <<
+ T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
+ writel(val, tegra->sata_regs + SCFG_OFFSET +
+ T_SATA0_CHX_PHY_CTRL1_GEN2);
+
+ writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
+ tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
+ writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
+ tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
+
+ writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
+
+ /* Program controller device ID */
+
+ val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+ val |= T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
+ writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+
+ writel(0x01060100, tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
+
+ val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+ val &= ~T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
+ writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
+
+ /* Enable IO & memory access, bus master mode */
+
+ val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
+ val |= T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
+ T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR;
+ writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
+
+ /* Program SATA MMIO */
+
+ writel(0x10000 << SATA_FPCI_BAR5_START_SHIFT,
+ tegra->sata_regs + SATA_FPCI_BAR5);
+
+ writel(0x08000 << T_SATA0_CFG_9_BASE_ADDRESS_SHIFT,
+ tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
+
+ /* Unmask SATA interrupts */
+
+ val = readl(tegra->sata_regs + SATA_INTR_MASK);
+ val |= SATA_INTR_MASK_IP_INT_MASK;
+ writel(val, tegra->sata_regs + SATA_INTR_MASK);
+
+ return 0;
+}
+
+static void tegra_ahci_controller_deinit(struct ahci_host_priv *hpriv)
+{
+ tegra_ahci_power_off(hpriv);
+}
+
+static void tegra_ahci_host_stop(struct ata_host *host)
+{
+ struct ahci_host_priv *hpriv = host->private_data;
+
+ tegra_ahci_controller_deinit(hpriv);
+}
+
+static struct ata_port_operations ahci_tegra_port_ops = {
+ .inherits = &ahci_ops,
+ .host_stop = tegra_ahci_host_stop,
+};
+
+static const struct ata_port_info ahci_tegra_port_info = {
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_tegra_port_ops,
+};
+
+static const struct of_device_id tegra_ahci_of_match[] = {
+ { .compatible = "nvidia,tegra124-ahci" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, tegra_ahci_of_match);
+
+static int tegra_ahci_probe(struct platform_device *pdev)
+{
+ struct ahci_host_priv *hpriv;
+ struct tegra_ahci_priv *tegra;
+ struct resource *res;
+ int ret;
+
+ hpriv = ahci_platform_get_resources(pdev);
+ if (IS_ERR(hpriv))
+ return PTR_ERR(hpriv);
+
+ tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
+ if (!tegra)
+ return -ENOMEM;
+
+ hpriv->plat_data = tegra;
+
+ tegra->pdev = pdev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ tegra->sata_regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(tegra->sata_regs))
+ return PTR_ERR(tegra->sata_regs);
+
+ tegra->sata_rst = devm_reset_control_get(&pdev->dev, "sata");
+ if (IS_ERR(tegra->sata_rst)) {
+ dev_err(&pdev->dev, "Failed to get sata reset\n");
+ return PTR_ERR(tegra->sata_rst);
+ }
+
+ tegra->sata_oob_rst = devm_reset_control_get(&pdev->dev, "sata-oob");
+ if (IS_ERR(tegra->sata_oob_rst)) {
+ dev_err(&pdev->dev, "Failed to get sata-oob reset\n");
+ return PTR_ERR(tegra->sata_oob_rst);
+ }
+
+ tegra->sata_cold_rst = devm_reset_control_get(&pdev->dev, "sata-cold");
+ if (IS_ERR(tegra->sata_cold_rst)) {
+ dev_err(&pdev->dev, "Failed to get sata-cold reset\n");
+ return PTR_ERR(tegra->sata_cold_rst);
+ }
+
+ tegra->sata_clk = devm_clk_get(&pdev->dev, "sata");
+ if (IS_ERR(tegra->sata_clk)) {
+ dev_err(&pdev->dev, "Failed to get sata clock\n");
+ return PTR_ERR(tegra->sata_clk);
+ }
+
+ tegra->supplies[0].supply = "avdd";
+ tegra->supplies[1].supply = "hvdd";
+ tegra->supplies[2].supply = "vddio";
+ tegra->supplies[3].supply = "target-5v";
+ tegra->supplies[4].supply = "target-12v";
+
+ ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra->supplies),
+ tegra->supplies);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to get regulators\n");
+ return ret;
+ }
+
+ ret = tegra_ahci_controller_init(hpriv);
+ if (ret)
+ return ret;
+
+ ret = ahci_platform_init_host(pdev, hpriv, &ahci_tegra_port_info,
+ 0, 0, 0);
+ if (ret)
+ goto deinit_controller;
+
+ return 0;
+
+deinit_controller:
+ tegra_ahci_controller_deinit(hpriv);
+
+ return ret;
+};
+
+static struct platform_driver tegra_ahci_driver = {
+ .probe = tegra_ahci_probe,
+ .remove = ata_platform_remove_one,
+ .driver = {
+ .name = "tegra-ahci",
+ .of_match_table = tegra_ahci_of_match,
+ },
+ /* LP0 suspend support not implemented */
+};
+module_platform_driver(tegra_ahci_driver);
+
+MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
+MODULE_DESCRIPTION("Tegra124 AHCI SATA driver");
+MODULE_LICENSE("GPL v2");
--
1.8.1.5
^ permalink raw reply related
* [PATCH v5 1/8] of: Add NVIDIA Tegra SATA controller binding
From: Mikko Perttunen @ 2014-07-18 7:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405500863-19696-2-git-send-email-mperttunen@nvidia.com>
This patch adds device tree binding documentation for the SATA
controller found on NVIDIA Tegra SoCs.
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v5: remove ordering requirement again
.../devicetree/bindings/ata/tegra-sata.txt | 30 ++++++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ata/tegra-sata.txt
diff --git a/Documentation/devicetree/bindings/ata/tegra-sata.txt b/Documentation/devicetree/bindings/ata/tegra-sata.txt
new file mode 100644
index 0000000..946f207
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/tegra-sata.txt
@@ -0,0 +1,30 @@
+Tegra124 SoC SATA AHCI controller
+
+Required properties :
+- compatible : "nvidia,tegra124-ahci".
+- reg : Should contain 2 entries:
+ - AHCI register set (SATA BAR5)
+ - SATA register set
+- interrupts : Defines the interrupt used by SATA
+- clocks : Must contain an entry for each entry in clock-names.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries:
+ - sata
+ - sata-oob
+ - cml1
+ - pll_e
+- resets : Must contain an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+- reset-names : Must include the following entries:
+ - sata
+ - sata-oob
+ - sata-cold
+- phys : Must contain an entry for each entry in phy-names.
+ See ../phy/phy-bindings.txt for details.
+- phy-names : Must include the following entries:
+ - sata-phy : XUSB PADCTL SATA PHY
+- hvdd-supply : Defines the SATA HVDD regulator
+- vddio-supply : Defines the SATA VDDIO regulator
+- avdd-supply : Defines the SATA AVDD regulator
+- target-5v-supply : Defines the SATA 5V power regulator
+- target-12v-supply : Defines the SATA 12V power regulator
--
1.8.1.5
^ permalink raw reply related
* [RFC PATCH] usb: dwc3: core: allow vendor drivers to check probe status
From: Lee Jones @ 2014-07-18 7:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140717172045.GM10459@saruman.home>
On Thu, 17 Jul 2014, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jul 17, 2014 at 06:13:33PM +0100, Lee Jones wrote:
> > This patch provides mechanism for subordinate devices to check
> > whether the DWC3 core probed successfully or otherwise. Useful
> > if PHYs are required to configure controllers, but aren't yet
> > available. The DWC3 core driver will defer probe if PHYs are
> > unavailable, however subordinate DWC3 drivers currently do not
> > have any visibility or means to check status - until now.
>
> what's a subordinate DWC3 driver ?
>
> > Another way to do this would be to *_phy_get*(), but if every
> > driver did this it would create a high level of code
> > duplication.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > drivers/usb/dwc3/core.c | 12 ++++++++++++
> > drivers/usb/dwc3/core.h | 1 +
> > 2 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index eb69eb9..171ca52 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -47,6 +47,14 @@
> >
> > /* -------------------------------------------------------------------------- */
> >
> > +static bool is_enabled = false;
> > +
> > +int dwc3_is_enabled(void)
> > +{
> > + return is_enabled;
> > +}
> > +EXPORT_SYMBOL(dwc3_is_enabled);
>
> no, no, no, no. Let me try that again, hello no! You _do_ realise there
> are systems with more than one dwc3 instance, right ? And this is the
> most fragile possible way of doing this.
>
> You never explained what's a dwc3 subordinate driver, you don't show any
> example of how this would be used and why/where does the PHY need to
> poke into DWC3. Why isn't probe defer enough for you ? Which platform
> are you working on ? what is the problem that you're trying to solve ?
>
> From this patch, all I can is NAK this patch with no mercy, sorry.
That's okay, I knew this was going to happen hence the RFC status of
the patch. In the DT case, I describe 'subordinate devices' as are
drivers which register the DWC3 core using of_platform_populate(),
so, for now:
drivers/usb/dwc3/dwc3-exynos.c
drivers/usb/dwc3/dwc3-keystone.c
drivers/usb/dwc3/dwc3-omap.c
We're attempting to use the same process; however, at the moment we are
suffering with a 'boot order' issue. If the PHYs aren't up and we
attempt to configure through the glue-layer our board locks up.
Presumably waiting for a read to return, forever. Whist the core does
the correct thing i.e. -EPROBE_DEFER, we (dwc3-st.c) have no way of
checking the return status of dwc3_probe(). As mentioned in the
commit message, another way of ensuring the PHYs are available is to
request them, but this would mean an awful lot of code duplication.
In your opinion, what's the best way to handle this?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 1/5] arm: dts: omap3-gta04: Add missing nodes to fully describe gta04 board
From: Joachim Eastwood @ 2014-07-18 6:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0C2AB07A-C515-4D5E-A823-DDF7C871515A@goldelico.com>
On 16 July 2014 09:17, Dr. H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Am 15.07.2014 um 14:45 schrieb Joachim Eastwood:
>
>> Hi Marek,
>>
>> You seem to add some DT nodes for hw that doesn't have drivers in
>> mainline. I think you should leave those out until the driver itself
>> is upstream and the bindings for it is documented.
> is there some policy for only having nodes for existing drivers in DT files?
I am not sure about that, better ask the DT maintainers.
There is however a policy that says new bindings should be reviewed
and documented.
> If I understand the device tree concept correctly, it should not describe drivers
> (and hence nothing about the state of them being mainlined), but it should statically
> describe the given hardware in a way that kernel and drivers can read it (or ignore).
> And even other kernels can use it (because they run on the same hardware).
>
> So unless there is an important reason not to have "currently unused" nodes
> in the DT source/binary (loading time is IMHO neglectable), I would ask that we
> can keep with the complete DT instead of splitting it up artificially and getting back
> to the same status (because the hardware does not change over time).
The problem with the nodes is that the bindings may change when the
drivers/bindings are submitted to mainline and reviewed. I don't see
why you want nodes for hardware that right now obviously don't work
with mainline and if the bindings change it will churn and confusion
for the users.
One example is the omap-pwm driver. It should really have #pwm-cells
set to 3 and not 2. This driver is not upstream yet, because it
requires some files to move out of arch/arm. See
http://marc.info/?l=linux-omap&m=139903867022779&w=2 for the suggested
new bindings.
The normal case for new DT boards is to first submit nodes for the
hardware that is working in mainline and then update the DT with more
nodes for hardware as bindings are decided upon and drivers go
upstream.
Please don't top post.
regards,
Joachim Eastwood
>> On 14 July 2014 22:20, Marek Belisko <marek@goldelico.com> wrote:
>>> Signed-off-by: Marek Belisko <marek@goldelico.com>
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>> ---
>>> arch/arm/boot/dts/omap3-gta04.dts | 443 +++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 412 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3-gta04.dts b/arch/arm/boot/dts/omap3-gta04.dts
>>> index 215513b..bd6a71d 100644
>>> --- a/arch/arm/boot/dts/omap3-gta04.dts
>>> +++ b/arch/arm/boot/dts/omap3-gta04.dts
>>> @@ -12,7 +12,7 @@
>>> #include "omap36xx.dtsi"
>>>
>>> / {
>>> - model = "OMAP3 GTA04";
>>> + model = "Goldelico GTA04";
>>> compatible = "ti,omap3-gta04", "ti,omap36xx", "ti,omap3";
>>>
>>> cpus {
>>> @@ -26,6 +26,11 @@
>>> reg = <0x80000000 0x20000000>; /* 512 MB */
>>> };
>>>
>>> + aliases {
>>> + display0 = &lcd;
>>> + display1 = &tv0;
>>> + };
>>> +
>>> gpio-keys {
>>> compatible = "gpio-keys";
>>>
>>> @@ -37,15 +42,78 @@
>>> };
>>> };
>>>
>>> + gpio-keys-wwan-wakeup {
>>> + compatible = "gpio-keys";
>>> +
>>> + wwan_wakeup_button: wwan-wakeup-button {
>>> + label = "3G_WOE";
>>> + linux,code = <240>;
>>> + gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
>>> + gpio-key,wakeup;
>>> + };
>>> + };
>>> +
>>> + hsusb2_phy: hsusb2_phy {
>>> + compatible = "usb-nop-xceiv";
>>> + reset-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>; /* gpio_174 = reset for USB3322 */
>>> + };
>>> +
>>> + antenna-detect {
>>> + compatible = "linux,extcon-gpio";
>>> + label = "gps_antenna";
>>> + gpios = <&gpio5 16 0>; /* gpio_144 */
>>> + debounce-delay-ms = <10>;
>>> + irq-flags = <IRQ_TYPE_EDGE_BOTH>;
>>> + state-on = "external";
>>> + state-off = "internal";
>>> + };
>>> +
>>> sound {
>>> compatible = "ti,omap-twl4030";
>>> ti,model = "gta04";
>>>
>>> ti,mcbsp = <&mcbsp2>;
>>> ti,codec = <&twl_audio>;
>>> +
>>> + ti,mcbsp-voice = <&mcbsp4>;
>>> + };
>>> +
>>> + sound_card {
>>> + compatible = "goldelico,gta04-audio";
>>> + gta04,cpu-dai = <&mcbsp2>;
>>> + };
>>> +
>>> + gtm601_codec: voice_codec {
>>> + compatible = "gtm601-codec";
>>> + };
>>> +
>>> + sound_voice {
>>> + compatible = "goldelico,gta04-voice";
>>> + gta04,cpu-dai = <&mcbsp4>;
>>> + gta04,codec = <>m601_codec>;
>>> };
>>>
>>> - spi_lcd {
>>> + w2cbw003_codec: headset_codec {
>>> + compatible = "w2cbw003-codec";
>>> + };
>>> +
>>> + sound_headset {
>>> + compatible = "goldelico,gta04-headset";
>>> + gta04,cpu-dai = <&mcbsp3>;
>>> + gta04,codec = <&w2cbw003_codec>;
>>> + };
>>> +
>>> + sound_fm {
>>> + compatible = "goldelico,gta04-fm";
>>> + gta04,cpu-dai = <&mcbsp1>;
>>> + gta04,codec = <&si4721_codec>;
>>> + };
>>> +
>>> + madc-hwmon {
>>> + compatible = "ti,twl4030-madc-hwmon";
>>> + };
>>> +
>>> + spi_lcd: spi_lcd {
>>> compatible = "spi-gpio";
>>> #address-cells = <0x1>;
>>> #size-cells = <0x0>;
>>> @@ -75,7 +143,7 @@
>>> };
>>> };
>>>
>>> - battery {
>>> + madc_battery: battery {
>>> compatible = "ti,twl4030-madc-battery";
>>> capacity = <1200000>;
>>> charging-calibration-data = <4200 100
>>> @@ -100,6 +168,83 @@
>>> "ichg",
>>> "vbat";
>>> };
>>> +
>>> + backlight {
>>> + compatible = "pwm-backlight";
>>> + pwms = <&pwm 0 2000000>;
>>> + brightness-levels = <0 11 20 30 40 50 60 70 80 90 100>;
>>> + default-brightness-level = <10>;
>>> + pinctrl-names = "default";
>>> + pintcrl-0 = <&backlight_pins>;
>>> + power-supply = <&power>;
>>> + };
>>> +
>>> + pwm: omap_pwm {
>>> + compatible = "ti,omap-pwm";
>>> + timers = <&timer11>;
>>> + #pwm-cells = <2>;
>>> + };
>>
>> The omap-pwm driver is not yet upstream and thus the bindings are not final.
>>
>>> + /* should be a PWM */
>>> + power: fixed_regulator at 0 {
>>> + compatible = "regulator-fixed";
>>> + regulator-name = "bl-enable";
>>> + regulator-boot-on;
>>> + regulator-always-on;
>>> + regulator-min-microvolt = "1000000";
>>> + regulator-max-microvolt = "1000000";
>>> + gpio = <&gpio2 25 0>; /* GPT11/PWM */
>>> + enable-active-high;
>>> + };
>>> +
>>> + tv0: connector at 1 {
>>> + compatible = "svideo-connector";
>>> + label = "tv";
>>> +
>>> + port {
>>> + tv_connector_in: endpoint {
>>> + remote-endpoint = <&venc_out>;
>>> + };
>>> + };
>>> + };
>>> +
>>> + tv_amp: opa362 {
>>> + compatible = "ti,opa362";
>>> + gpio = <&gpio1 23 0>; /* GPIO to enable video out amplifier */
>>> + };
>>
>> Driver for a OPAMP?
>>
>>> + /* presents a single gpio to be plumbed to uart1 dts */
>>> + bt_en: w2cbw003 {
>>> + compatible = "wi2wi,w2cbw003";
>>> + gpio-controller;
>>> + #gpio-cells = <2>;
>>> +
>>> + vdd-supply = <&vaux4>;
>>> + };
>>> +
>>> + /* presents a single gpio to be plumbed to uart2 dts */
>>> + gps_en: w2sg0004 {
>>> + compatible = "wi2wi,w2sg0004";
>>> + gpio-controller;
>>> + #gpio-cells = <2>;
>>> +
>>> + lna-supply = <&vsim>; /* LNA regulator */
>>> + on-off-gpio = <&gpio5 17 0>; /* gpio_145: trigger for turning on/off w2sg0004 */
>>> + rx-gpio = <&gpio5 19 0>; /* gpio_147: RX */
>>> + rx-on-mux = < (PIN_INPUT_PULLUP | MUX_MODE0) >;
>>> + rx-off-mux = < (PIN_INPUT_PULLUP | MUX_MODE4) >;
>>> + };
>>> +
>>> + /* control modem power through rfkill */
>>> + modem_en: modem {
>>> + compatible = "option,gtm601";
>>> + gpio-controller;
>>> + #gpio-cells = <2>;
>>> +
>>> + usb-port = <&hsusb2_phy>;
>>> + on-off-gpio = <&gpio6 26 0>; /* gpio_186: trigger to power on modem */
>>> + on-indicator-gpio = <0>;
>>> + };
>>> };
>>
>> I don't think any of the above devices have documented bindings so I
>> don't think it is a good idea to add the nodes now.
>>
>>> &omap3_pmx_core {
>>> @@ -168,11 +313,72 @@
>>>> ;
>>> };
>>>
>>> + backlight_pins: backlight_pins_pimnux {
>>> + pinctrl-single,pins = <0x8a MUX_MODE4>;
>>> + };
>>> +
>>> + hsusb2_pins: pinmux_hsusb2_pins {
>>> + pinctrl-single,pins = <
>>> + OMAP3_CORE1_IOPAD(0x21d4, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi1_cs3.hsusb2_data2 */
>>> + OMAP3_CORE1_IOPAD(0x21d6, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_clk.hsusb2_data7 */
>>> + OMAP3_CORE1_IOPAD(0x21d8, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_simo.hsusb2_data4 */
>>> + OMAP3_CORE1_IOPAD(0x21da, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_somi.hsusb2_data5 */
>>> + OMAP3_CORE1_IOPAD(0x21dc, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_cs0.hsusb2_data6 */
>>> + OMAP3_CORE1_IOPAD(0x21de, PIN_INPUT_PULLDOWN | MUX_MODE3) /* mcspi2_cs1.hsusb2_data3 */
>>> + >;
>>> + };
>>> +
>>> + i2c1_pins: pinmux_i2c1_pins {
>>> + pinctrl-single,pins = <
>>> + 0x18a (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_scl.i2c1_scl */
>>> + 0x18c (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_sda.i2c1_sda */
>>> + >;
>>> + };
>>> +
>>> + i2c2_pins: pinmux_i2c2_pins {
>>> + pinctrl-single,pins = <
>>> + 0x18e (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c2_scl.i2c2_scl */
>>> + 0x190 (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c2_sda.i2c2_sda */
>>> + >;
>>> + };
>>> +
>>> + i2c3_pins: pinmux_i2c3_pins {
>>> + pinctrl-single,pins = <
>>> + 0x192 (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c3_scl.i2c3_scl */
>>> + 0x194 (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c3_sda.i2c3_sda */
>>> + >;
>>> + };
>>
>> Any reason why you don't use OMAP_IOPAD macro for the I2C pins?
>>
>>> +
>>> + bma180_pins: pinmux_bma180_pins {
>>> + pinctrl-single,pins = <
>>> + OMAP3_CORE1_IOPAD(0x213a, PIN_INPUT_PULLUP | MUX_MODE4) /* gpio_115 */
>>> + >;
>>> + };
>>> +};
>>> +
>>> +&omap3_pmx_core2 {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <
>>> + &hsusb2_2_pins
>>> + >;
>>> +
>>> + hsusb2_2_pins: pinmux_hsusb2_2_pins {
>>> + pinctrl-single,pins = <
>>> + OMAP3630_CORE2_IOPAD(0x25f0, PIN_OUTPUT | MUX_MODE3) /* etk_d10.hsusb2_clk */
>>> + OMAP3630_CORE2_IOPAD(0x25f2, PIN_OUTPUT | MUX_MODE3) /* etk_d11.hsusb2_stp */
>>> + OMAP3630_CORE2_IOPAD(0x25f4, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d12.hsusb2_dir */
>>> + OMAP3630_CORE2_IOPAD(0x25f6, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d13.hsusb2_nxt */
>>> + OMAP3630_CORE2_IOPAD(0x25f8, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d14.hsusb2_data0 */
>>> + OMAP3630_CORE2_IOPAD(0x25fa, PIN_INPUT_PULLDOWN | MUX_MODE3) /* etk_d15.hsusb2_data1 */
>>> + >;
>>> + };
>>> +
>>> spi_gpio_pins: spi_gpio_pinmux {
>>> - pinctrl-single,pins = <0x5a8 (PIN_OUTPUT | MUX_MODE4) /* clk */
>>> - 0x5b6 (PIN_OUTPUT | MUX_MODE4) /* cs */
>>> - 0x5b8 (PIN_OUTPUT | MUX_MODE4) /* tx */
>>> - 0x5b4 (PIN_INPUT | MUX_MODE4) /* rx */
>>> + pinctrl-single,pins = <
>>> + OMAP3630_CORE2_IOPAD(0x25d8, PIN_OUTPUT | MUX_MODE4) /* clk */
>>> + OMAP3630_CORE2_IOPAD(0x25e6, PIN_OUTPUT | MUX_MODE4) /* cs */
>>> + OMAP3630_CORE2_IOPAD(0x25e8, PIN_OUTPUT | MUX_MODE4) /* tx */
>>> + OMAP3630_CORE2_IOPAD(0x25e4, PIN_INPUT | MUX_MODE4) /* rx */
>>>> ;
>>> };
>>> };
>>
>> regards,
>> Joachim Eastwood
>
^ permalink raw reply
* [PATCH v13 7/8] arm64: add pmd_[dirty|mkclean] for THP
From: Minchan Kim @ 2014-07-18 6:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405666386-15095-1-git-send-email-minchan@kernel.org>
MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
overwrite of the contents since MADV_FREE syscall is called for
THP page.
This patch adds pmd_dirty and pmd_mkclean for THP page MADV_FREE
support.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Steve Capper <steve.capper@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
arch/arm64/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index e0ccceb317d9..41dfe3c7260c 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -240,10 +240,12 @@ static inline pmd_t pte_pmd(pte_t pte)
#endif
#define pmd_young(pmd) pte_young(pmd_pte(pmd))
+#define pmd_dirty(pmd) pte_dirty(pmd_pte(pmd))
#define pmd_wrprotect(pmd) pte_pmd(pte_wrprotect(pmd_pte(pmd)))
#define pmd_mksplitting(pmd) pte_pmd(pte_mkspecial(pmd_pte(pmd)))
#define pmd_mkold(pmd) pte_pmd(pte_mkold(pmd_pte(pmd)))
#define pmd_mkwrite(pmd) pte_pmd(pte_mkwrite(pmd_pte(pmd)))
+#define pmd_mkclean(pmd) pte_pmd(pte_mkclean(pmd_pte(pmd)))
#define pmd_mkdirty(pmd) pte_pmd(pte_mkdirty(pmd_pte(pmd)))
#define pmd_mkyoung(pmd) pte_pmd(pte_mkyoung(pmd_pte(pmd)))
#define pmd_mknotpresent(pmd) (__pmd(pmd_val(pmd) & ~PMD_TYPE_MASK))
--
2.0.0
^ permalink raw reply related
* [PATCH v13 6/8] arm: add pmd_[dirty|mkclean] for THP
From: Minchan Kim @ 2014-07-18 6:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405666386-15095-1-git-send-email-minchan@kernel.org>
MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
overwrite of the contents since MADV_FREE syscall is called for
THP page.
This patch adds pmd_dirty and pmd_mkclean for THP page MADV_FREE
support.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Steve Capper <steve.capper@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
arch/arm/include/asm/pgtable-3level.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
index 85c60adc8b60..830f84f2d277 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -220,6 +220,8 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)
#define pmd_trans_splitting(pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING)
#endif
+#define pmd_dirty(pmd) (pmd_val(pmd) & PMD_SECT_DIRTY)
+
#define PMD_BIT_FUNC(fn,op) \
static inline pmd_t pmd_##fn(pmd_t pmd) { pmd_val(pmd) op; return pmd; }
@@ -228,6 +230,7 @@ PMD_BIT_FUNC(mkold, &= ~PMD_SECT_AF);
PMD_BIT_FUNC(mksplitting, |= PMD_SECT_SPLITTING);
PMD_BIT_FUNC(mkwrite, &= ~PMD_SECT_RDONLY);
PMD_BIT_FUNC(mkdirty, |= PMD_SECT_DIRTY);
+PMD_BIT_FUNC(mkclean, &= ~PMD_SECT_DIRTY);
PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);
#define pmd_mkhuge(pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))
--
2.0.0
^ permalink raw reply related
* [v3 PATCH 1/6] drivers: reset: TI: SoC reset controller support.
From: Lothar Waßmann @ 2014-07-18 6:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405615531-15649-1-git-send-email-dmurphy@ti.com>
Hi,
Dan Murphy wrote:
> The TI SoC reset controller support utilizes the
> reset controller framework to give device drivers or
> function drivers a common set of APIs to call to reset
> a module.
>
> The reset-ti is a common interface to the reset framework.
> The register data is retrieved during initialization
> of the reset driver through the reset-ti-data
> file. The array of data is associated with the compatible from the
> respective DT entry.
>
> Once the data is available then this is derefenced within the common
> interface.
>
> The device driver has the ability to assert, deassert or perform a
> complete reset.
>
> This code was derived from previous work by Rajendra Nayak and Afzal Mohammed.
> The code was changed to adopt to the reset core and abstract away the SoC information.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v3 - Resolved comments from v2. To many to call out here - https://patchwork.kernel.org/patch/4116941/
>
> drivers/reset/Kconfig | 9 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-ti.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 383 insertions(+)
> create mode 100644 drivers/reset/reset-ti.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 0615f50..31a5a79 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
>
> If unsure, say no.
>
> +config RESET_TI
> + depends on RESET_CONTROLLER && ARCH_OMAP || COMPILE_TEST
> + bool "TI reset controller"
> + help
> + Reset controller support for TI SoC's
> +
> + Reset controller found in TI's AM series of SoC's like
> + AM335x and AM43x and OMAP SoC's like OMAP5 and DRA7
> +
> source "drivers/reset/sti/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..a5986b9 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_RESET_CONTROLLER) += core.o
> obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI) += reset-ti.o
> obj-$(CONFIG_ARCH_STI) += sti/
> diff --git a/drivers/reset/reset-ti.c b/drivers/reset/reset-ti.c
> new file mode 100644
> index 0000000..e9d4039
> --- /dev/null
> +++ b/drivers/reset/reset-ti.c
> @@ -0,0 +1,373 @@
> +/*
> + * reset-ti.c - PRCM reset driver for TI SoC's
> + *
> + * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com
> + *
> + * Author: Dan Murphy <dmurphy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define DRIVER_NAME "prcm_reset_ti"
> +#define MAX_RESET_SIGNALS 255
> +
> +/**
> + * struct ti_reset_reg_data - Structure of the reset register information
> + * for a particular SoC.
> + * @rstctrl_offs: This is the reset control offset value from
> + * from the parent reset node.
> + * @rstst_offs: This is the reset status offset value from
> + * from the parent reset node.
> + * @rstctrl_bit: This is the reset control bit for the module.
> + * @rstst_bit: This is the reset status bit for the module.
> + *
> + * Longer description of this structure.
> + */
> +struct ti_reset_reg_data {
> + phandle handle;
> + u32 rstctrl_offs;
> + u32 rstst_offs;
> + u32 rstctrl_bit;
> + u32 rstst_bit;
> +};
> +
> +/**
> + * struct ti_reset_data - Structure that contains the reset register data
> + * as well as the total number of resets for a particular SoC.
> + * @ti_data: Pointer to this structure to be dereferenced
> + * @reg_data: Pointer to the register data structure.
> + * @rcdev: Reset controller device instance
> + * @dev: Pointer to the devive structure
> + * @ti_reg_data: Array of register data. Only reset signal with valid
> + * phandles will be stored in this array.
> + * @reg_base: Parent register base address
> + * @lock: Spinlock for accessing the registers
> + * @nr_resets: Total number of resets for the SoC in the reset array.
> + *
> + * This structure contains a pointer to the register data and the modules
> + * register base. The number of resets and reset controller device data is
> + * stored within this structure.
> + *
> + */
> +struct ti_reset_data {
> + struct ti_reset_data *ti_data;
> + struct ti_reset_reg_data *reg_data;
> + struct reset_controller_dev rcdev;
> + struct device *dev;
> + struct ti_reset_reg_data ti_reg_data[MAX_RESET_SIGNALS];
> + void __iomem *reg_base;
> + spinlock_t lock;
> + int nr_resets;
> +};
> +
> +static int ti_reset_wait_on_reset(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct ti_reset_data *reset_data = container_of(rcdev,
> + struct ti_reset_data,
> + rcdev);
> + void __iomem *status_reg = reset_data->reg_base;
> + u32 bit_mask = 0;
> + u32 val = 0;
> + unsigned long flags;
> + int i = 1000;
> + int ret = 0;
> +
> + spin_lock_irqsave(&reset_data->lock, flags);
> + /* Clear the reset status bit to reflect the current status */
> + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> + bit_mask = reset_data->ti_reg_data[id].rstst_bit;
> +
> + do {
> + val = readl(status_reg);
> + } while (--i && (val & (1 << bit_mask)));
> +
Is there no better way to detect timeout than counting down from some
magic number? Does the reference manual give any hints about how fast
the reset should be completed, so the timeout can be hinged on some
real microseconds rather than a number of CPU loops?
> + if (i == 0 && val & (1 << bit_mask)) {
> + dev_err(reset_data->dev, "%s: Reset failed\n", __func__);
> + ret = -EIO;
> + }
> +
> + spin_unlock_irqrestore(&reset_data->lock, flags);
> + return ret;
> +}
> +
> +static int ti_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct ti_reset_data *reset_data = container_of(rcdev,
> + struct ti_reset_data,
> + rcdev);
> + void __iomem *reg;
> + void __iomem *status_reg;
> + u32 status_bit = 0;
> + u32 bit_mask = 0;
> + u32 val = 0;
>
Useless initialization!
> + unsigned long flags;
> +
> + spin_lock_irqsave(&reset_data->lock, flags);
> +
> + /* Clear the reset status bit to reflect the current status */
> + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> + status_bit = reset_data->ti_reg_data[id].rstst_bit;
> + writel(1 << status_bit, status_reg);
> +
> + reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> + bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> + val = readl(reg);
> + if (!(val & bit_mask)) {
> + val |= bit_mask;
> + writel(val, reg);
> + }
> +
> + spin_unlock_irqrestore(&reset_data->lock, flags);
> + return 0;
> +}
> +
> +static int ti_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> +
> + struct ti_reset_data *reset_data = container_of(rcdev,
> + struct ti_reset_data,
> + rcdev);
> + void __iomem *reg;
> + void __iomem *status_reg;
> + u32 status_bit = 0;
> + u32 bit_mask = 0;
> + u32 val = 0;
>
dto.
> + unsigned long flags;
> +
> + spin_lock_irqsave(&reset_data->lock, flags);
> +
> + /* Clear the reset status bit to reflect the current status */
> + status_reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstst_offs;
> + status_bit = reset_data->ti_reg_data[id].rstst_bit;
> + writel(1 << status_bit, status_reg);
> +
> + reg = reset_data->reg_base + reset_data->ti_reg_data[id].rstctrl_offs;
> + bit_mask = reset_data->ti_reg_data[id].rstctrl_bit;
> + val = readl(reg);
> + if (val & bit_mask) {
> + val &= ~bit_mask;
> + writel(val, reg);
> + }
> +
> + spin_unlock_irqrestore(&reset_data->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ti_reset_reset(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + ti_reset_assert(rcdev, id);
> + ti_reset_deassert(rcdev, id);
> +
> + return ti_reset_wait_on_reset(rcdev, id);
> +}
> +
> +static int ti_reset_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> +
> + struct ti_reset_data *reset_data = container_of(rcdev,
> + struct ti_reset_data,
> + rcdev);
> + struct device_node *dev_node;
> + int i;
> +
> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> + return -EINVAL;
> +
> + /* Verify that the phandle exists */
> + dev_node = of_find_node_by_phandle((phandle) reset_spec->args[0]);
> + if (!dev_node) {
> + dev_err(reset_data->dev,
> + "%s: Cannot find phandle node\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + for (i = 0; i <= reset_data->nr_resets; i++) {
> + if (reset_data->ti_reg_data[i].handle == dev_node->phandle)
> + return i;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static struct reset_control_ops ti_reset_ops = {
> + .reset = ti_reset_reset,
> + .assert = ti_reset_assert,
> + .deassert = ti_reset_deassert,
> +};
> +
> +static int ti_reset_populate_child(struct ti_reset_data *reset_data,
> + struct device_node *reset_child,
> + u32 ctrl_offs, u32 status_offs)
> +{
> + struct device_node *next_reset_child = NULL;
> + int i = reset_data->nr_resets;
> + int ret;
> + u32 ctrl_bit;
> +
> + while ((next_reset_child = of_get_next_child(reset_child, next_reset_child))) {
> + if (next_reset_child->phandle) {
> + /* Get the bits of each sub-reset */
> + ret = of_property_read_u32(next_reset_child, "control-bit",
> + &ctrl_bit);
> + if (ret < 0) {
> + dev_err(reset_data->dev,
> + "%s: No entry in %s for rstctrl_offs\n",
> + __func__, next_reset_child->name);
> +
> + return -ENODEV;
>
return the error code from of_property_read()?
> + }
> +
> + ret = of_property_read_u32(next_reset_child, "status-bit",
> + &reset_data->ti_reg_data[i].rstst_bit);
> + if (ret < 0) {
> + dev_err(reset_data->dev,
> + "%s: No entry in %s for rstst_offs\n",
> + __func__, next_reset_child->name);
> +
> + return -ENODEV;
>
dto.
> + }
> +
> + reset_data->ti_reg_data[i].rstctrl_offs = ctrl_offs;
> + reset_data->ti_reg_data[i].rstst_offs = status_offs;
> + reset_data->ti_reg_data[i].handle = next_reset_child->phandle;
> + i++;
> + }
> + };
> +
> + reset_data->nr_resets = i;
> +
> + return 0;
> +}
> +
> +static int ti_reset_get_of_data(struct ti_reset_data *reset_data,
> + struct device_node *dev_node)
> +{
> + struct device_node *reset_child = NULL;
> + int ret = -EINVAL;
>
useless initialization.
> + u32 ctrl_offs, status_offs;
> +
> + /* Loop through all the children and populate a lookup array */
> + while ((reset_child = of_get_next_child(dev_node, reset_child))) {
>
I'd prefer:
while ((reset_child = of_get_next_child(dev_node,
reset_child)) != NULL) { to make it abundantly clear that the = is not meant as a comparison
operator but an assignment.
> + ret = of_property_read_u32_index(reset_child, "reg", 0,
> + &ctrl_offs);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_u32_index(reset_child, "reg", 1,
> + &status_offs);
> + if (ret)
> + return ret;
> +
> + ret = ti_reset_populate_child(reset_data, reset_child,
> + ctrl_offs, status_offs);
> + if (ret)
> + return ret;
> + };
> +
> + return 0;
> +}
> +
> +static int ti_reset_probe(struct platform_device *pdev)
> +{
> + struct device_node *resets;
> + struct ti_reset_data *reset_data;
> + struct resource *res;
> + int ret;
> +
> + reset_data = devm_kzalloc(&pdev->dev, sizeof(*reset_data), GFP_KERNEL);
> + if (!reset_data)
> + return -ENOMEM;
> +
> + resets = of_find_node_by_name(NULL, "resets");
> + if (!resets) {
> + dev_err(&pdev->dev, "%s: missing 'resets' child node.\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reset_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(reset_data->reg_base)) {
> + dev_err(&pdev->dev, "%s: Cannot map reset parent.\n", __func__);
> + return -ENODEV;
>
return the error code from devm_ioremap_resource.
> + }
> +
> + spin_lock_init(&reset_data->lock);
> + reset_data->dev = &pdev->dev;
> + ret = ti_reset_get_of_data(reset_data, resets);
> + if (ret)
> + return -EINVAL;
>
return ret!
> + reset_data->rcdev.owner = THIS_MODULE;
> + reset_data->rcdev.of_node = resets;
> + reset_data->rcdev.ops = &ti_reset_ops;
> +
> + reset_data->rcdev.of_reset_n_cells = 1;
> + reset_data->rcdev.of_xlate = &ti_reset_xlate;
> +
> + reset_data->ti_data = reset_data;
> +
> + platform_set_drvdata(pdev, reset_data);
> +
> + reset_controller_register(&reset_data->rcdev);
> +
> + return 0;
> +}
> +
> +static int ti_reset_remove(struct platform_device *pdev)
> +{
> + struct ti_reset_data *reset_data;
> +
> + reset_data = platform_get_drvdata(pdev);
>
struct ti_reset_data *reset_data = platform_get_drvdata(pdev);
> + reset_controller_unregister(&reset_data->rcdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ti_reset_of_match[] = {
> + { .compatible = "ti,omap5-prm" },
> + { .compatible = "ti,omap4-prm" },
> + { .compatible = "ti,omap5-prm" },
> + { .compatible = "ti,dra7-prm" },
> + { .compatible = "ti,am4-prcm" },
> + { .compatible = "ti,am3-prcm" },
> + {},
>
useless comma after list terminator.
Without the comma an entry inadvertedly added after the terminator
(e.g. due to a badly resolved merge conflict) will lead to a compile
error while otherwise the new entry will silently be ignored.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply
* [RFC] cpufreq: Add bindings for CPU clock sharing topology
From: Viresh Kumar @ 2014-07-18 6:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMhTkTEXp99gpU8cZBwYmiXpsySG5zMn6oSyo_Mf5PAiUg@mail.gmail.com>
On 18 July 2014 11:47, Olof Johansson <olof@lixom.net> wrote:
> Why complicate it by using two properties?
>
> If there is no property, then the CPUs are assumed to be controlled
> independently.
>
> if there is a clock-master = <phandle> property, then that points at
> the cpu that is the main one controlling clock for the group.
>
> There's really no need to label the master -- it will be the only one
> with incoming links but nothing outgoing. And a master without slaves
> is an independently controlled cpu so you should be fine in that
> aspect too.
I thought so earlier, but then I remembered something I read long back.
Don't remember which thread now, but I *might* be wrong..
"Bindings are like APIs and new bindings shouldn't break existing stuff.."
And:
> If there is no property, then the CPUs are assumed to be controlled
> independently.
seems to break the existing API.
But if that isn't the case, the bindings are very simple & clear to handle.
Diff for new bindings:
diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
index 30ce9ad..d127bba 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
@@ -9,15 +9,14 @@ Clock lines may or may not be shared among different
CPUs on a platform.
separate line for itself
Optional Properties:
-- clock-master: This declares cpu as clock master. Other CPUs can either define
- "clock-ganged" or "clock-master" property, but shouldn't be missing both.
+- clock-master: Contains phandle of the master cpu controlling clocks.
-- clock-ganged: Should have phandle of a cpu declared as "clock-master".
-
-If any cpu node, doesn't have both "clock-master" and "clock-ganged" properties
-defined, it would be assumed that all CPUs on that platform share a
single clock
-line. This will help supporting already upstreamed platforms.
+ Ideally there is nothing like a "master" CPU as any CPU can play with DVFS
+ settings. But we have to choose one cpu out of a group, so that others can
+ point to it.
+ If there is no "clock-master" property for a cpu node, it is considered as
+ master. It may or may not have other slave CPUs pointing towards it.
Examples:
1.) All CPUs share a single clock line
@@ -30,34 +29,6 @@ cpus {
compatible = "arm,cortex-a15";
reg = <0>;
next-level-cache = <&L2>;
- clock-master;
- operating-points = <
- /* kHz uV */
- 792000 1100000
- 396000 950000
- 198000 850000
- >;
- clock-latency = <61036>; /* two CLK32 periods */
- };
-
- cpu1: cpu at 1 {
- compatible = "arm,cortex-a15";
- reg = <1>;
- next-level-cache = <&L2>;
- clock-ganged = <&cpu0>;
- };
-};
-
-OR (clock-master/ganged aren't defined)
-
-cpus {
- #address-cells = <1>;
- #size-cells = <0>;
-
- cpu0: cpu at 0 {
- compatible = "arm,cortex-a15";
- reg = <0>;
- next-level-cache = <&L2>;
operating-points = <
/* kHz uV */
792000 1100000
@@ -71,6 +42,7 @@ cpus {
compatible = "arm,cortex-a15";
reg = <1>;
next-level-cache = <&L2>;
+ clock-master = <&cpu0>;
};
};
@@ -83,7 +55,6 @@ cpus {
compatible = "arm,cortex-a15";
reg = <0>;
next-level-cache = <&L2>;
- clock-master;
operating-points = <
/* kHz uV */
792000 1100000
@@ -97,7 +68,6 @@ cpus {
compatible = "arm,cortex-a15";
reg = <1>;
next-level-cache = <&L2>;
- clock-master;
operating-points = <
/* kHz uV */
792000 1100000
@@ -119,7 +89,6 @@ cpus {
compatible = "arm,cortex-a15";
reg = <0>;
next-level-cache = <&L2>;
- clock-master;
operating-points = <
/* kHz uV */
792000 1100000
@@ -133,14 +102,13 @@ cpus {
compatible = "arm,cortex-a15";
reg = <1>;
next-level-cache = <&L2>;
- clock-ganged = <&cpu0>;
+ clock-master = <&cpu0>;
};
cpu2: cpu at 100 {
compatible = "arm,cortex-a7";
reg = <100>;
next-level-cache = <&L2>;
- clock-master;
operating-points = <
/* kHz uV */
792000 950000
@@ -154,6 +122,6 @@ cpus {
compatible = "arm,cortex-a7";
reg = <101>;
next-level-cache = <&L2>;
- clock-ganged = <&cpu2>;
+ clock-master = <&cpu2>;
};
};
^ permalink raw reply related
* [PATCH 5/5] tty: serial: Add 8250-core based omap driver
From: Tony Lindgren @ 2014-07-18 6:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53C7A01C.50107@linutronix.de>
* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140717 03:09]:
> On 07/17/2014 10:12 AM, Tony Lindgren wrote:
> > Hmm it could be that it works for a while because the clocks are on
> > from the bootloader and pm_runtime calls won't do anything. This
> > could happen if the interconnect data based on the ti,hwmods entry
> > is not getting matched to the new driver. This gets initialized when
> > the device entry gets created in omap_device_build_from_dt().
> >
> > Or maybe something now affects the clock aliases? It seems that we
> > are still missing the clocks entries in the .dtsi files, see the
> > mappings with $ git grep uart drivers/clk/ti/
>
> I've been looking for something completely different while I noticed
> this:
>
> in drivers/tty/serial/omap-serial.c
> | static struct platform_driver serial_omap_driver = {
> | .driver = {
> | .name = DRIVER_NAME,
> | },
> | };
> |
>
> and DRIVER_NAME should come from include/linux/platform_data/serial-omap.h
> Looking further, I've found arch/arm/mach-omap2/serial.c:
> | void __init omap_serial_init_port(struct omap_board_data *bdata,
> | struct omap_uart_port_info *info)
> | {
> | char *name
> ?
> | name = DRIVER_NAME;
> ?
> | pdev = omap_device_build(name, uart->num, oh, pdata, pdata_size);
> ?
> |
>
> Would this explain it?
That would explain it for legacy booting, but not for device tree
based booting. I can try to debug it further on Monday.
Regards,
Tony
^ permalink raw reply
* [RFC] cpufreq: Add bindings for CPU clock sharing topology
From: Olof Johansson @ 2014-07-18 6:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7e097b71342c9f5f63b07ff2e135eb7beb626aab.1405661369.git.viresh.kumar@linaro.org>
On Thu, Jul 17, 2014 at 10:35 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Clock lines may or may not be shared among different CPUs on a platform. When
> clock lines are shared between CPUs, they change DVFS state together.
>
> Possible configurations:
>
> 1.) All CPUs share a single clock line.
> 2.) All CPUs have independent clock lines.
> 3.) CPUs within a group/cluster share clock line but each group/cluster have a
> separate line for itself.
>
> There is no generic way available today to detect which CPUs share clock lines
> and so this is an attempt towards that.
>
> Much of the information is present in the commit and so no point duplicating it
> here.
>
> These are obviously not finalized yet and this is an attempt to initiate a
> discussion around this.
>
> Please share your valuable feedback.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> I wasn't sure about the path/name of this file, so please don't blast me on that
> :)
>
> .../devicetree/bindings/cpufreq/cpu_clocks.txt | 159 +++++++++++++++++++++
> 1 file changed, 159 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
> new file mode 100644
> index 0000000..30ce9ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpu_clocks.txt
> @@ -0,0 +1,159 @@
> +* Generic CPUFreq clock bindings
> +
> +Clock lines may or may not be shared among different CPUs on a platform.
> +
> +Possible configurations:
> +1.) All CPUs share a single clock line
> +2.) All CPUs have independent clock lines
> +3.) CPUs within a group/cluster share clock line but each group/cluster have a
> + separate line for itself
> +
> +Optional Properties:
> +- clock-master: This declares cpu as clock master. Other CPUs can either define
> + "clock-ganged" or "clock-master" property, but shouldn't be missing both.
> +
> +- clock-ganged: Should have phandle of a cpu declared as "clock-master".
Why complicate it by using two properties?
If there is no property, then the CPUs are assumed to be controlled
independently.
if there is a clock-master = <phandle> property, then that points at
the cpu that is the main one controlling clock for the group.
There's really no need to label the master -- it will be the only one
with incoming links but nothing outgoing. And a master without slaves
is an independently controlled cpu so you should be fine in that
aspect too.
-Olof
^ permalink raw reply
* [PATCH] ARM: rockchip: Add cpu hotplug support for RK3XXX SoCs
From: Romain Perier @ 2014-07-18 6:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140717221436.GZ21766@n2100.arm.linux.org.uk>
Hi all,
you're probably talking about __cpu_die at
http://lxr.free-electrons.com/source/arch/arm/kernel/smp.c#L223 . The
problem being that the thread below which executes cpu_die completes the
completion before executing smp_ops.cpu_die. So smp_ops.cpu_kill might
be executed before smp_ops.cpu_die (in some cases cache coherency would
not be turned off). Note that almost all SoCs do the same thing.
On 18/07/2014 00:14, Russell King - ARM Linux wrote:
> On Thu, Jul 17, 2014 at 05:11:50PM +0000, Romain Perier wrote:
>> +static DECLARE_COMPLETION(cpu_died);
>> +
>> +static int rockchip_cpu_kill(unsigned int cpu)
>> +{
>> + if (!wait_for_completion_timeout(&cpu_died, msecs_to_jiffies(1000))) {
>> + pr_err("CPU%d: didn't die correctly\n", cpu);
>> + return 0;
>> + }
> Please explain the point of this cpu_died thing.
>
> I think you should read arch/arm/kernel/smp.c, searching for cpu_died
> in there.
>
^ permalink raw reply
* [PATCHv6 4/4] ARM: dts: Fix wrong compatible string for Exynos3250 ADC
From: Chanwoo Choi @ 2014-07-18 5:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405663186-26464-1-git-send-email-cw00.choi@samsung.com>
This patchset fix wrong compatible string for Exynos3250 ADC. Exynos3250 SoC
need to control only special clock for ADC. Exynos SoC except for Exynos3250
has not included special clock for ADC. The exynos ADC driver can control
special clock if compatible string is 'exynos3250-adc-v2'.
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
---
arch/arm/boot/dts/exynos3250.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 3d3c45b..e039818 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -391,10 +391,10 @@
};
adc: adc at 126C0000 {
- compatible = "samsung,exynos-adc-v3";
+ compatible = "samsung,exynos3250-adc-v2";
reg = <0x126C0000 0x100>, <0x10020718 0x4>;
interrupts = <0 137 0>;
- clock-names = "adc", "sclk_tsadc";
+ clock-names = "adc", "sclk_adc";
clocks = <&cmu CLK_TSADC>, <&cmu CLK_SCLK_TSADC>;
#io-channel-cells = <1>;
io-channel-ranges;
--
1.8.0
^ permalink raw reply related
* [PATCHv6 3/4] iio: devicetree: Add DT binding documentation for Exynos3250 ADC
From: Chanwoo Choi @ 2014-07-18 5:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405663186-26464-1-git-send-email-cw00.choi@samsung.com>
This patch add DT binding documentation for Exynos3250 ADC IP. Exynos3250 has
special clock ('sclk_adc') for ADC which provide clock to internal ADC.
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
---
.../devicetree/bindings/arm/samsung/exynos-adc.txt | 25 ++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index 832fe8c..26232f9 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -14,14 +14,21 @@ Required properties:
for exynos4412/5250 controllers.
Must be "samsung,exynos-adc-v2" for
future controllers.
+ Must be "samsung,exynos3250-adc-v2" for
+ controllers compatible with ADC of Exynos3250.
- reg: Contains ADC register address range (base address and
length) and the address of the phy enable register.
- interrupts: Contains the interrupt information for the timer. The
format is being dependent on which interrupt controller
the Samsung device uses.
- #io-channel-cells = <1>; As ADC has multiple outputs
-- clocks From common clock binding: handle to adc clock.
-- clock-names From common clock binding: Shall be "adc".
+- clocks From common clock bindings: handles to clocks specified
+ in "clock-names" property, in the same order.
+- clock-names From common clock bindings: list of clock input names
+ used by ADC block:
+ - "adc" : ADC bus clock
+ - "sclk_adc" : ADC special clock (only for Exynos3250
+ and compatible ADC block)
- vdd-supply VDD input supply.
Note: child nodes can be added for auto probing from device tree.
@@ -41,6 +48,20 @@ adc: adc at 12D10000 {
vdd-supply = <&buck5_reg>;
};
+Example: adding device info in dtsi file for Exynos3250 with additional sclk
+
+adc: adc at 126C0000 {
+ compatible = "samsung,exynos3250-adc-v2";
+ reg = <0x126C0000 0x100>, <0x10020718 0x4>;
+ interrupts = <0 137 0>;
+ #io-channel-cells = <1>;
+ io-channel-ranges;
+
+ clocks = <&cmu CLK_TSADC>, <&cmu CLK_SCLK_TSADC>;
+ clock-names = "adc", "sclk_adc";
+
+ vdd-supply = <&buck5_reg>;
+};
Example: Adding child nodes in dts file
--
1.8.0
^ permalink raw reply related
* [PATCHv6 2/4] iio: adc: exynos_adc: Control special clock of ADC to support Exynos3250 ADC
From: Chanwoo Choi @ 2014-07-18 5:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405663186-26464-1-git-send-email-cw00.choi@samsung.com>
This patch control special clock for ADC in Exynos series's FSYS block.
If special clock of ADC is registerd on clock list of common clk framework,
Exynos ADC drvier have to control this clock.
Exynos3250/Exynos4/Exynos5 has 'adc' clock as following:
- 'adc' clock: bus clock for ADC
Exynos3250 has additional 'sclk_adc' clock as following:
- 'sclk_adc' clock: special clock for ADC which provide clock to internal ADC
Exynos 4210/4212/4412 and Exynos5250/5420 has not included 'sclk_adc' clock
in FSYS_BLK. But, Exynos3250 based on Cortex-A7 has only included 'sclk_adc'
clock in FSYS_BLK.
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
drivers/iio/adc/exynos_adc.c | 112 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 101 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 00d67fd..b63e882 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -81,9 +81,11 @@
struct exynos_adc {
struct exynos_adc_data *data;
+ struct device *dev;
void __iomem *regs;
void __iomem *enable_reg;
struct clk *clk;
+ struct clk *sclk;
unsigned int irq;
struct regulator *vdd;
@@ -95,6 +97,7 @@ struct exynos_adc {
struct exynos_adc_data {
int num_channels;
+ bool needs_sclk;
void (*init_hw)(struct exynos_adc *info);
void (*exit_hw)(struct exynos_adc *info);
@@ -102,6 +105,66 @@ struct exynos_adc_data {
void (*start_conv)(struct exynos_adc *info, unsigned long addr);
};
+static void exynos_adc_unprepare_clk(struct exynos_adc *info)
+{
+ if (info->data->needs_sclk)
+ clk_unprepare(info->sclk);
+ clk_unprepare(info->clk);
+}
+
+static int exynos_adc_prepare_clk(struct exynos_adc *info)
+{
+ int ret;
+
+ ret = clk_prepare(info->clk);
+ if (ret) {
+ dev_err(info->dev, "failed preparing adc clock: %d\n", ret);
+ return ret;
+ }
+
+ if (info->data->needs_sclk) {
+ ret = clk_prepare(info->sclk);
+ if (ret) {
+ clk_unprepare(info->clk);
+ dev_err(info->dev,
+ "failed preparing sclk_adc clock: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static void exynos_adc_disable_clk(struct exynos_adc *info)
+{
+ if (info->data->needs_sclk)
+ clk_disable(info->sclk);
+ clk_disable(info->clk);
+}
+
+static int exynos_adc_enable_clk(struct exynos_adc *info)
+{
+ int ret;
+
+ ret = clk_enable(info->clk);
+ if (ret) {
+ dev_err(info->dev, "failed enabling adc clock: %d\n", ret);
+ return ret;
+ }
+
+ if (info->data->needs_sclk) {
+ ret = clk_enable(info->sclk);
+ if (ret) {
+ clk_disable(info->clk);
+ dev_err(info->dev,
+ "failed enabling sclk_adc clock: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static void exynos_adc_v1_init_hw(struct exynos_adc *info)
{
u32 con1;
@@ -199,13 +262,20 @@ static void exynos_adc_v2_start_conv(struct exynos_adc *info,
writel(con1 | ADC_CON_EN_START, ADC_V2_CON1(info->regs));
}
+#define __EXYNOS_ADC_V2_DATA \
+ .num_channels = MAX_ADC_V2_CHANNELS, \
+ .init_hw = exynos_adc_v2_init_hw, \
+ .exit_hw = exynos_adc_v2_exit_hw, \
+ .clear_irq = exynos_adc_v2_clear_irq, \
+ .start_conv = exynos_adc_v2_start_conv, \
+
static struct exynos_adc_data const exynos_adc_v2_data = {
- .num_channels = MAX_ADC_V2_CHANNELS,
+ __EXYNOS_ADC_V2_DATA
+};
- .init_hw = exynos_adc_v2_init_hw,
- .exit_hw = exynos_adc_v2_exit_hw,
- .clear_irq = exynos_adc_v2_clear_irq,
- .start_conv = exynos_adc_v2_start_conv,
+static struct exynos_adc_data const exynos3250_adc_v2_data = {
+ __EXYNOS_ADC_V2_DATA
+ .needs_sclk = true,
};
static const struct of_device_id exynos_adc_match[] = {
@@ -215,6 +285,9 @@ static const struct of_device_id exynos_adc_match[] = {
}, {
.compatible = "samsung,exynos-adc-v2",
.data = (void *)&exynos_adc_v2_data,
+ }, {
+ .compatible = "samsung,exynos3250-adc-v2",
+ .data = (void *)&exynos3250_adc_v2_data,
},
{},
};
@@ -376,6 +449,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
}
info->irq = irq;
+ info->dev = &pdev->dev;
init_completion(&info->completion);
@@ -386,6 +460,16 @@ static int exynos_adc_probe(struct platform_device *pdev)
return PTR_ERR(info->clk);
}
+ if (info->data->needs_sclk) {
+ info->sclk = devm_clk_get(&pdev->dev, "sclk_adc");
+ if (IS_ERR(info->sclk)) {
+ dev_err(&pdev->dev,
+ "failed getting sclk_adc, err = %ld\n",
+ PTR_ERR(info->sclk));
+ return PTR_ERR(info->sclk);
+ }
+ }
+
info->vdd = devm_regulator_get(&pdev->dev, "vdd");
if (IS_ERR(info->vdd)) {
dev_err(&pdev->dev, "failed getting regulator, err = %ld\n",
@@ -397,10 +481,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = clk_prepare_enable(info->clk);
+ ret = exynos_adc_prepare_clk(info);
if (ret)
goto err_disable_reg;
+ ret = exynos_adc_enable_clk(info);
+ if (ret)
+ goto err_unprepare_clk;
+
platform_set_drvdata(pdev, indio_dev);
indio_dev->name = dev_name(&pdev->dev);
@@ -443,7 +531,9 @@ err_irq:
err_disable_clk:
if (info->data->exit_hw)
info->data->exit_hw(info);
- clk_disable_unprepare(info->clk);
+ exynos_adc_disable_clk(info);
+err_unprepare_clk:
+ exynos_adc_unprepare_clk(info);
err_disable_reg:
regulator_disable(info->vdd);
return ret;
@@ -460,7 +550,8 @@ static int exynos_adc_remove(struct platform_device *pdev)
free_irq(info->irq, info);
if (info->data->exit_hw)
info->data->exit_hw(info);
- clk_disable_unprepare(info->clk);
+ exynos_adc_disable_clk(info);
+ exynos_adc_unprepare_clk(info);
regulator_disable(info->vdd);
return 0;
@@ -474,8 +565,7 @@ static int exynos_adc_suspend(struct device *dev)
if (info->data->exit_hw)
info->data->exit_hw(info);
-
- clk_disable_unprepare(info->clk);
+ exynos_adc_disable_clk(info);
regulator_disable(info->vdd);
return 0;
@@ -491,7 +581,7 @@ static int exynos_adc_resume(struct device *dev)
if (ret)
return ret;
- ret = clk_prepare_enable(info->clk);
+ ret = exynos_adc_enable_clk(info);
if (ret)
return ret;
--
1.8.0
^ permalink raw reply related
* [PATCHv6 1/4] iio: adc: exynos_adc: Add exynos_adc_data structure to improve readability
From: Chanwoo Choi @ 2014-07-18 5:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405663186-26464-1-git-send-email-cw00.choi@samsung.com>
This patchset add 'exynos_adc_data' structure which includes some functions
to control ADC operation and specific data according to ADC version (v1 or v2).
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
drivers/iio/adc/exynos_adc.c | 226 ++++++++++++++++++++++++++++---------------
1 file changed, 147 insertions(+), 79 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 010578f..00d67fd 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -39,11 +39,6 @@
#include <linux/iio/machine.h>
#include <linux/iio/driver.h>
-enum adc_version {
- ADC_V1,
- ADC_V2
-};
-
/* EXYNOS4412/5250 ADC_V1 registers definitions */
#define ADC_V1_CON(x) ((x) + 0x00)
#define ADC_V1_DLY(x) ((x) + 0x08)
@@ -85,6 +80,7 @@ enum adc_version {
#define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
struct exynos_adc {
+ struct exynos_adc_data *data;
void __iomem *regs;
void __iomem *enable_reg;
struct clk *clk;
@@ -97,43 +93,139 @@ struct exynos_adc {
unsigned int version;
};
-static const struct of_device_id exynos_adc_match[] = {
- { .compatible = "samsung,exynos-adc-v1", .data = (void *)ADC_V1 },
- { .compatible = "samsung,exynos-adc-v2", .data = (void *)ADC_V2 },
- {},
+struct exynos_adc_data {
+ int num_channels;
+
+ void (*init_hw)(struct exynos_adc *info);
+ void (*exit_hw)(struct exynos_adc *info);
+ void (*clear_irq)(struct exynos_adc *info);
+ void (*start_conv)(struct exynos_adc *info, unsigned long addr);
};
-MODULE_DEVICE_TABLE(of, exynos_adc_match);
-static inline unsigned int exynos_adc_get_version(struct platform_device *pdev)
+static void exynos_adc_v1_init_hw(struct exynos_adc *info)
{
- const struct of_device_id *match;
+ u32 con1;
- match = of_match_node(exynos_adc_match, pdev->dev.of_node);
- return (unsigned int)match->data;
+ writel(1, info->enable_reg);
+
+ /* set default prescaler values and Enable prescaler */
+ con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
+
+ /* Enable 12-bit ADC resolution */
+ con1 |= ADC_V1_CON_RES;
+ writel(con1, ADC_V1_CON(info->regs));
+}
+
+static void exynos_adc_v1_exit_hw(struct exynos_adc *info)
+{
+ u32 con;
+
+ writel(0, info->enable_reg);
+
+ con = readl(ADC_V1_CON(info->regs));
+ con |= ADC_V1_CON_STANDBY;
+ writel(con, ADC_V1_CON(info->regs));
+}
+
+static void exynos_adc_v1_clear_irq(struct exynos_adc *info)
+{
+ writel(1, ADC_V1_INTCLR(info->regs));
}
-static void exynos_adc_hw_init(struct exynos_adc *info)
+static void exynos_adc_v1_start_conv(struct exynos_adc *info,
+ unsigned long addr)
+{
+ u32 con1;
+
+ writel(addr, ADC_V1_MUX(info->regs));
+
+ con1 = readl(ADC_V1_CON(info->regs));
+ writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
+}
+
+static struct exynos_adc_data const exynos_adc_v1_data = {
+ .num_channels = MAX_ADC_V1_CHANNELS,
+
+ .init_hw = exynos_adc_v1_init_hw,
+ .exit_hw = exynos_adc_v1_exit_hw,
+ .clear_irq = exynos_adc_v1_clear_irq,
+ .start_conv = exynos_adc_v1_start_conv,
+};
+
+static void exynos_adc_v2_init_hw(struct exynos_adc *info)
{
u32 con1, con2;
- if (info->version == ADC_V2) {
- con1 = ADC_V2_CON1_SOFT_RESET;
- writel(con1, ADC_V2_CON1(info->regs));
+ writel(1, info->enable_reg);
- con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
- ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
- writel(con2, ADC_V2_CON2(info->regs));
+ con1 = ADC_V2_CON1_SOFT_RESET;
+ writel(con1, ADC_V2_CON1(info->regs));
- /* Enable interrupts */
- writel(1, ADC_V2_INT_EN(info->regs));
- } else {
- /* set default prescaler values and Enable prescaler */
- con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
+ con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
+ ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
+ writel(con2, ADC_V2_CON2(info->regs));
- /* Enable 12-bit ADC resolution */
- con1 |= ADC_V1_CON_RES;
- writel(con1, ADC_V1_CON(info->regs));
- }
+ /* Enable interrupts */
+ writel(1, ADC_V2_INT_EN(info->regs));
+}
+
+static void exynos_adc_v2_exit_hw(struct exynos_adc *info)
+{
+ u32 con;
+
+ writel(0, info->enable_reg);
+
+ con = readl(ADC_V2_CON1(info->regs));
+ con &= ~ADC_CON_EN_START;
+ writel(con, ADC_V2_CON1(info->regs));
+}
+
+static void exynos_adc_v2_clear_irq(struct exynos_adc *info)
+{
+ writel(1, ADC_V2_INT_ST(info->regs));
+}
+
+static void exynos_adc_v2_start_conv(struct exynos_adc *info,
+ unsigned long addr)
+{
+ u32 con1, con2;
+
+ con2 = readl(ADC_V2_CON2(info->regs));
+ con2 &= ~ADC_V2_CON2_ACH_MASK;
+ con2 |= ADC_V2_CON2_ACH_SEL(addr);
+ writel(con2, ADC_V2_CON2(info->regs));
+
+ con1 = readl(ADC_V2_CON1(info->regs));
+ writel(con1 | ADC_CON_EN_START, ADC_V2_CON1(info->regs));
+}
+
+static struct exynos_adc_data const exynos_adc_v2_data = {
+ .num_channels = MAX_ADC_V2_CHANNELS,
+
+ .init_hw = exynos_adc_v2_init_hw,
+ .exit_hw = exynos_adc_v2_exit_hw,
+ .clear_irq = exynos_adc_v2_clear_irq,
+ .start_conv = exynos_adc_v2_start_conv,
+};
+
+static const struct of_device_id exynos_adc_match[] = {
+ {
+ .compatible = "samsung,exynos-adc-v1",
+ .data = (void *)&exynos_adc_v1_data,
+ }, {
+ .compatible = "samsung,exynos-adc-v2",
+ .data = (void *)&exynos_adc_v2_data,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, exynos_adc_match);
+
+static struct exynos_adc_data *exynos_adc_get_data(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+
+ match = of_match_node(exynos_adc_match, pdev->dev.of_node);
+ return (struct exynos_adc_data *)match->data;
}
static int exynos_read_raw(struct iio_dev *indio_dev,
@@ -144,7 +236,6 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
{
struct exynos_adc *info = iio_priv(indio_dev);
unsigned long timeout;
- u32 con1, con2;
int ret;
if (mask != IIO_CHAN_INFO_RAW)
@@ -154,28 +245,15 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
reinit_completion(&info->completion);
/* Select the channel to be used and Trigger conversion */
- if (info->version == ADC_V2) {
- con2 = readl(ADC_V2_CON2(info->regs));
- con2 &= ~ADC_V2_CON2_ACH_MASK;
- con2 |= ADC_V2_CON2_ACH_SEL(chan->address);
- writel(con2, ADC_V2_CON2(info->regs));
-
- con1 = readl(ADC_V2_CON1(info->regs));
- writel(con1 | ADC_CON_EN_START,
- ADC_V2_CON1(info->regs));
- } else {
- writel(chan->address, ADC_V1_MUX(info->regs));
-
- con1 = readl(ADC_V1_CON(info->regs));
- writel(con1 | ADC_CON_EN_START,
- ADC_V1_CON(info->regs));
- }
+ if (info->data->start_conv)
+ info->data->start_conv(info, chan->address);
timeout = wait_for_completion_timeout
(&info->completion, EXYNOS_ADC_TIMEOUT);
if (timeout == 0) {
dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
- exynos_adc_hw_init(info);
+ if (info->data->init_hw)
+ info->data->init_hw(info);
ret = -ETIMEDOUT;
} else {
*val = info->value;
@@ -193,13 +271,11 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
struct exynos_adc *info = (struct exynos_adc *)dev_id;
/* Read value */
- info->value = readl(ADC_V1_DATX(info->regs)) &
- ADC_DATX_MASK;
+ info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
+
/* clear irq */
- if (info->version == ADC_V2)
- writel(1, ADC_V2_INT_ST(info->regs));
- else
- writel(1, ADC_V1_INTCLR(info->regs));
+ if (info->data->clear_irq)
+ info->data->clear_irq(info);
complete(&info->completion);
@@ -277,6 +353,12 @@ static int exynos_adc_probe(struct platform_device *pdev)
info = iio_priv(indio_dev);
+ info->data = exynos_adc_get_data(pdev);
+ if (!info->data) {
+ dev_err(&pdev->dev, "failed getting exynos_adc_data\n");
+ return -EINVAL;
+ }
+
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
info->regs = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(info->regs))
@@ -319,10 +401,6 @@ static int exynos_adc_probe(struct platform_device *pdev)
if (ret)
goto err_disable_reg;
- writel(1, info->enable_reg);
-
- info->version = exynos_adc_get_version(pdev);
-
platform_set_drvdata(pdev, indio_dev);
indio_dev->name = dev_name(&pdev->dev);
@@ -331,11 +409,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
indio_dev->info = &exynos_adc_iio_info;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = exynos_adc_iio_channels;
-
- if (info->version == ADC_V1)
- indio_dev->num_channels = MAX_ADC_V1_CHANNELS;
- else
- indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
+ indio_dev->num_channels = info->data->num_channels;
ret = request_irq(info->irq, exynos_adc_isr,
0, dev_name(&pdev->dev), info);
@@ -349,7 +423,8 @@ static int exynos_adc_probe(struct platform_device *pdev)
if (ret)
goto err_irq;
- exynos_adc_hw_init(info);
+ if (info->data->init_hw)
+ info->data->init_hw(info);
ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
if (ret < 0) {
@@ -366,7 +441,8 @@ err_of_populate:
err_irq:
free_irq(info->irq, info);
err_disable_clk:
- writel(0, info->enable_reg);
+ if (info->data->exit_hw)
+ info->data->exit_hw(info);
clk_disable_unprepare(info->clk);
err_disable_reg:
regulator_disable(info->vdd);
@@ -382,7 +458,8 @@ static int exynos_adc_remove(struct platform_device *pdev)
exynos_adc_remove_devices);
iio_device_unregister(indio_dev);
free_irq(info->irq, info);
- writel(0, info->enable_reg);
+ if (info->data->exit_hw)
+ info->data->exit_hw(info);
clk_disable_unprepare(info->clk);
regulator_disable(info->vdd);
@@ -394,19 +471,10 @@ static int exynos_adc_suspend(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct exynos_adc *info = iio_priv(indio_dev);
- u32 con;
- if (info->version == ADC_V2) {
- con = readl(ADC_V2_CON1(info->regs));
- con &= ~ADC_CON_EN_START;
- writel(con, ADC_V2_CON1(info->regs));
- } else {
- con = readl(ADC_V1_CON(info->regs));
- con |= ADC_V1_CON_STANDBY;
- writel(con, ADC_V1_CON(info->regs));
- }
+ if (info->data->exit_hw)
+ info->data->exit_hw(info);
- writel(0, info->enable_reg);
clk_disable_unprepare(info->clk);
regulator_disable(info->vdd);
@@ -427,8 +495,8 @@ static int exynos_adc_resume(struct device *dev)
if (ret)
return ret;
- writel(1, info->enable_reg);
- exynos_adc_hw_init(info);
+ if (info->data->init_hw)
+ info->data->init_hw(info);
return 0;
}
--
1.8.0
^ permalink raw reply related
* [PATCHv6 0/4] iio: adc: exynos_adc: Support Exynos3250 ADC and code clean
From: Chanwoo Choi @ 2014-07-18 5:59 UTC (permalink / raw)
To: linux-arm-kernel
This patchset support Exynos3250 ADC (Analog Digital Converter) because
Exynos3250 has additional special clock for ADC IP.
Changes from v5:
- Add acked message by Kukjin Kim
- Add reviewed messgae by Tomasz Figa
- Fix typo (for for -> for)
Changes from v4:
- Use 'exynos_adc_data' structure instead of 'exynos_adc_ops' structure
and remove enum variable of ADC version
- Fix wrong name of special clock (sclk_tsadc -> sclk_adc)
- Add reviewed message by Naveen Krishna Chatradhi
- Add functions for ADC clock control
Changes from v3:
- Add new 'exynos_adc_ops' structure to improve readability according to
Tomasz Figa comment[1]
[1] https://lkml.org/lkml/2014/4/16/238
- Add new 'exynos3250-adc-v2' compatible string to support Exynos3250 ADC
- Fix wrong compaitlbe string of ADC in Exynos3250 dtsi file
Changes from v2:
- Check return value of clock function to deal with error exception
- Fix minor coding style to improve readability
Changes from v1:
- Add new "samsung,exynos-adc-v3" compatible to support Exynos3250 ADC
- Add a patch about DT binding documentation
Chanwoo Choi (4):
iio: adc: exynos_adc: Add exynos_adc_data structure to improve
readability
iio: adc: exynos_adc: Control special clock of ADC to support
Exynos3250 ADC
iio: devicetree: Add DT binding documentation for Exynos3250 ADC
ARM: dts: Fix wrong compatible string for Exynos3250 ADC
.../devicetree/bindings/arm/samsung/exynos-adc.txt | 25 +-
arch/arm/boot/dts/exynos3250.dtsi | 4 +-
drivers/iio/adc/exynos_adc.c | 326 +++++++++++++++------
3 files changed, 267 insertions(+), 88 deletions(-)
--
1.8.0
^ permalink raw reply
* [PATCH RFCv3 08/14] arm64: introduce aarch64_insn_gen_movewide()
From: Z Lim @ 2014-07-18 5:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140717094101.GD21153@arm.com>
(resending this email in case the first one got caught in your spam
filter. sorry.)
On Thu, Jul 17, 2014 at 10:41:02AM +0100, Will Deacon wrote:
> On Wed, Jul 16, 2014 at 11:04:22PM +0100, Zi Shen Lim wrote:
> > On Wed, Jul 16, 2014 at 05:17:15PM +0100, Will Deacon wrote:
> > > On Tue, Jul 15, 2014 at 07:25:06AM +0100, Zi Shen Lim wrote:
> > > > Introduce function to generate move wide (immediate) instructions.
[...]
> > > > + switch (variant) {
> > > > + case AARCH64_INSN_VARIANT_32BIT:
> > > > + BUG_ON(shift != 0 && shift != 16);
> > > > + break;
> > > > + case AARCH64_INSN_VARIANT_64BIT:
> > > > + insn |= BIT(31);
> > > > + BUG_ON(shift != 0 && shift != 16 && shift != 32 &&
> > > > + shift != 48);
> > >
> > > Would be neater as a nested switch, perhaps? If you reorder the
> > > outer-switch, you could probably fall-through too and combine the shift
> > > checks.
> >
> > Not sure I picture what you had in mind... I couldn't come up with a
> > neater version with the properties you described.
> >
> > The alternative I had was using masks instead of integer values, but
> > one could argue that while neater, it could also be harder to read:
> >
> > switch (variant) {
> > case AARCH64_INSN_VARIANT_32BIT:
> > BUG_ON(shift & ~BIT(4));
> > break;
> > case AARCH64_INSN_VARIANT_64BIT:
> > insn |= BIT(31);
> > BUG_ON(shift & ~GENMASK(5, 4));
> > ...
>
> I was thinking of using nested switches, but that doesn't fall out like I
> hoped. How about:
>
> switch (variant) {
> case AARCH64_INSN_VARIANT_64BIT:
> BUG_ON(shift != 32 && shift != 48);
Sorry this won't work. For example, on the valid case of shift==0,
we'll barf right here - no fallthrough.
Shall we just leave the code as is? :)
> case AARCH64_INSN_VARIANT_32BIT:
> BUG_ON(shift != 0 && shift != 16);
> };
>
> ?
>
> Will
^ permalink raw reply
* [PATCH RFCv3 01/14] arm64: introduce aarch64_insn_gen_comp_branch_imm()
From: Z Lim @ 2014-07-18 5:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140717172526.GC4844@arm.com>
(resending this email in case the first one got caught in your spam
filter. sorry.)
On Thu, Jul 17, 2014 at 06:25:26PM +0100, Will Deacon wrote:
> On Thu, Jul 17, 2014 at 04:59:10PM +0100, Alexei Starovoitov wrote:
> > On Thu, Jul 17, 2014 at 2:19 AM, Will Deacon <will.deacon@arm.com> wrote:
> > > On Wed, Jul 16, 2014 at 10:19:31PM +0100, Zi Shen Lim wrote:
> > >> >
> > >> > Is a BUG_ON justifiable here? Is there not a nicer way to fail?
> > >>
> > >> In general, it'd be nice if we returned something like -EINVAL and
> > >> have all callers handle failures. Today all code gen functions return
> > >> the u32 instruction and there's no error handling by callers.
> > >> I think following the precedence (aarch64_insn_gen_branch_imm())
> > >> of failing with BUG_ON is a reasonable tradeoff.
> > >
> > > Well, I don't necessarily agree with that BUG_ON, either :)
> > > I take it eBPF doesn't have a `trap' instruction or similar? Otherwise, we
> > > could generate that and avoid having to propagate errors directly to the
> > > caller.
> > >
> > >> In this case here, when we hit the default (failure) case, that means
> > >> there's a serious error of attempting to use an unsupported
> > >> variant. I think we're better off failing hard here than trying to
> > >> arbitrarily "fallback" on a default choice.
> > >
> > > It might be a serious error for BPF, but a BUG_ON brings down the entire
> > > machine, which I think is unfortunate.
> >
> > There is some misunderstanding here. Here BUG_ON will trigger
> > only on actual bug in JIT implementation, it cannot be triggered by user.
> > eBPF program is verified before it reaches JIT, so all instructions are
> > valid and input to JIT is proper. Two instruction are not yet
> > implemented in this JIT and they trigger pr_.._once().
> > So I don't see any issue with this usage of BUG_ON
> > imo living with silent bugs in JIT is more dangerous.
> >
> > For the same reason there is no 'trap' instruction in eBPF.
> > Static verifier checks that program is valid. If there was a 'trap'
> > insn the program would be rejected. Like programs with
> > 'div by zero' are rejected. There is normal 'bpf_exit' insn to
> > return from the program.
>
> Ok, so assuming that BPF doesn't have any issues, I take your point.
> However, we could very easily re-use these functions for things like SMP
> alternatives and kprobes, where simply failing the instruction generation
> might be acceptable.
>
> It just feels like a bit hammer to me, when the machine is probably happily
> scheduling user tasks, responding to interrupts, writing data to disk etc.
Yes I agree with you Will, it'd be truly unfortunate if we inadvertently
allow the entire system to be brought down.
Alexei accurately pointed out that if we ever hit such a case, it'd be a bug
in the BPF JIT implementation (or bug in other in-kernel implementations).
Our BPF JIT implementation actually handles this, making sure that input
to the codegen function is valid, or gracefully fail by not JITing and
falling back on the core BPF interpreter. This way our JIT will not trigger
the BUG_ON.
IMO, other future users of these codegen functions should do the same.
An alternative would be to throw away all the BUG_ON and have callers
check for and handle error conditions. I think this is actually more
dangerous as callers who don't handle the error conditions properly may
end up causing system crash later with subtle (and quite possibly hard to
debug) bugs.
>
> Will
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox