* [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
From: Kishon Vijay Abraham I @ 2014-01-21 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1385373690-12170-1-git-send-email-kishon@ti.com>
Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs),
do not return from probe if the USB PHY library returns -ENODEV as that
indicates the platform does not have PHY.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/usb/dwc3/core.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a49217a..e009d4e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -411,32 +411,26 @@ static int dwc3_probe(struct platform_device *pdev)
if (IS_ERR(dwc->usb2_phy)) {
ret = PTR_ERR(dwc->usb2_phy);
-
- /*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
- if (ret == -ENXIO)
+ if (ret == -ENXIO || ret == -ENODEV) {
+ dwc->usb2_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
return ret;
-
- dev_err(dev, "no usb2 phy configured\n");
- return -EPROBE_DEFER;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ return ret;
+ }
}
if (IS_ERR(dwc->usb3_phy)) {
ret = PTR_ERR(dwc->usb3_phy);
-
- /*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
- if (ret == -ENXIO)
+ if (ret == -ENXIO || ret == -ENODEV) {
+ dwc->usb3_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
return ret;
-
- dev_err(dev, "no usb3 phy configured\n");
- return -EPROBE_DEFER;
+ } else {
+ dev_err(dev, "no usb3 phy configured\n");
+ return ret;
+ }
}
dwc->xhci_resources[0].start = res->start;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down
From: Alexandre Courbot @ 2014-01-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-1-git-send-email-acourbot@nvidia.com>
Invoke the do_idle() firmware call before suspending a CPU so that the
underlying firmware (if any) can take necessary action.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/mach-tegra/cpuidle-tegra114.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index e0b87300243d..c42fd41065d2 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -19,6 +19,7 @@
#include <linux/cpuidle.h>
#include <linux/cpu_pm.h>
#include <linux/clockchips.h>
+#include <asm/firmware.h>
#include <asm/cpuidle.h>
#include <asm/suspend.h>
@@ -45,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ call_firmware_op(do_idle);
+
cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
--
1.8.5.3
^ permalink raw reply related
* [PATCH 4/5] ARM: trusted_foundations: implement do_idle()
From: Alexandre Courbot @ 2014-01-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-1-git-send-email-acourbot@nvidia.com>
Support the do_idle() firmware call, which is necessary to properly
support cpuidle.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/firmware/trusted_foundations.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index ef1e3d8f4af0..7c14af809995 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -22,6 +22,15 @@
#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+#define TF_CPU_PM 0xfffffffc
+#define TF_CPU_PM_LP0 0xffffffe3
+#define TF_CPU_PM_LP1 0xffffffe6
+#define TF_CPU_PM_LP1_NO_MC_CLK 0xffffffe5
+#define TF_CPU_PM_LP2 0xffffffe4
+#define TF_CPU_PM_LP2_NOFLUSH_L2 0xffffffe7
+
+static unsigned long cpu_boot_addr;
+
static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
{
asm volatile(
@@ -41,13 +50,22 @@ static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
- tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
+ cpu_boot_addr = boot_addr;
+ tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, cpu_boot_addr, 0);
+
+ return 0;
+}
+
+static int tf_do_idle(void)
+{
+ tf_generic_smc(TF_CPU_PM, TF_CPU_PM_LP2_NOFLUSH_L2, cpu_boot_addr);
return 0;
}
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
+ .do_idle = tf_do_idle,
};
void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
--
1.8.5.3
^ permalink raw reply related
* [PATCH 3/5] ARM: firmware: enable Trusted Foundations by default
From: Alexandre Courbot @ 2014-01-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-1-git-send-email-acourbot@nvidia.com>
As discussed previously (https://lkml.org/lkml/2013/11/26/289), enable
Trusted Foundation support by default since it already depends on a
supporting architecture being selected.
Doing so allows us to remove it from tegra_defconfig.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/configs/tegra_defconfig | 1 -
arch/arm/firmware/Kconfig | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 5fdc9a09d339..e38653876541 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -33,7 +33,6 @@ CONFIG_PCI=y
CONFIG_PCI_MSI=y
CONFIG_PCI_TEGRA=y
CONFIG_PCIEPORTBUS=y
-CONFIG_TRUSTED_FOUNDATIONS=y
CONFIG_SMP=y
CONFIG_PREEMPT=y
CONFIG_AEABI=y
diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
index bb126594995e..ad396af68e47 100644
--- a/arch/arm/firmware/Kconfig
+++ b/arch/arm/firmware/Kconfig
@@ -11,6 +11,7 @@ menu "Firmware options"
config TRUSTED_FOUNDATIONS
bool "Trusted Foundations secure monitor support"
depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+ default y
help
Some devices (including most Tegra-based consumer devices on the
market) are booted with the Trusted Foundations secure monitor
--
1.8.5.3
^ permalink raw reply related
* [PATCH 2/5] ARM: trusted_foundations: fallback when TF support is missing
From: Alexandre Courbot @ 2014-01-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-1-git-send-email-acourbot@nvidia.com>
When Trusted Foundations is detected as present on the system, but
Trusted Foundations support is not built into the kernel, the kernel
used to issue a panic very early during boot, leaving little clue to the
user as to what is going wrong.
It turns out that even without TF support built-in, the kernel can boot
on a TF-enabled system provided that SMP and cpuidle are disabled. This
patch does this and continue booting on one CPU, leaving the user with a
usable (however degraded) system.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/include/asm/trusted_foundations.h | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
index 997862fd5d77..b5f7705abcb0 100644
--- a/arch/arm/include/asm/trusted_foundations.h
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -30,6 +30,8 @@
#include <linux/printk.h>
#include <linux/bug.h>
#include <linux/of.h>
+#include <linux/cpu.h>
+#include <linux/smp.h>
struct trusted_foundations_platform_data {
unsigned int version_major;
@@ -47,10 +49,13 @@ static inline void register_trusted_foundations(
struct trusted_foundations_platform_data *pd)
{
/*
- * If we try to register TF, this means the system needs it to continue.
- * Its absence if thus a fatal error.
+ * If the system requires TF and we cannot provide it, continue booting
+ * but disable features that cannot be provided.
*/
- panic("No support for Trusted Foundations, stopping...\n");
+ pr_err("No support for Trusted Foundations, continuing in degraded mode.\n");
+ pr_err("Secondary processors as well as CPU PM will be disabled.\n");
+ setup_max_cpus = 0;
+ cpu_idle_poll_ctrl(true);
}
static inline void of_register_trusted_foundations(void)
--
1.8.5.3
^ permalink raw reply related
* [PATCH 1/5] ARM: trusted_foundations: fix vendor prefix typos
From: Alexandre Courbot @ 2014-01-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-1-git-send-email-acourbot@nvidia.com>
of_register_trusted_foundations() and the firmware Kconfig used
the wrong vendor prefix for Trusted Logic Mobility.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/firmware/Kconfig | 2 +-
arch/arm/include/asm/trusted_foundations.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
index bb00ccf00d66..bb126594995e 100644
--- a/arch/arm/firmware/Kconfig
+++ b/arch/arm/firmware/Kconfig
@@ -20,7 +20,7 @@ config TRUSTED_FOUNDATIONS
This option allows the kernel to invoke the secure monitor whenever
required on devices using Trusted Foundations. See
arch/arm/include/asm/trusted_foundations.h or the
- tl,trusted-foundations device tree binding documentation for details
+ tlm,trusted-foundations device tree binding documentation for details
on how to use it.
Say n if you don't know what this is about.
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
index 3bd36e2c5f2e..997862fd5d77 100644
--- a/arch/arm/include/asm/trusted_foundations.h
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -59,7 +59,7 @@ static inline void of_register_trusted_foundations(void)
* If we find the target should enable TF but does not support it,
* fail as the system won't be able to do much anyway
*/
- if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
+ if (of_find_compatible_node(NULL, NULL, "tlm,trusted-foundations"))
register_trusted_foundations(NULL);
}
#endif /* CONFIG_TRUSTED_FOUNDATIONS */
--
1.8.5.3
^ permalink raw reply related
* [PATCH 0/5] ARM: firmware: improvements to Trusted Foundations support
From: Alexandre Courbot @ 2014-01-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
These (mostly minor) patches fix a few typos, improve points that
were agreed upon when the Trusted Foundation series was initially
submitted, and more importantly add support for the do_idle() firmware
operation that is needed for cpuidle to be supported. Tegra's cpuidle
driver is also updated accordingly.
These patches should be the last step before the device trees for NVIDIA
SHIELD and Tegra Note 7 can be submitted.
Alexandre Courbot (5):
ARM: trusted_foundations: fix vendor prefix typos
ARM: trusted_foundations: fallback when TF support is missing
ARM: firmware: enable Trusted Foundations by default
ARM: trusted_foundations: implement do_idle()
ARM: tegra: cpuidle: use firmware call for power down
arch/arm/configs/tegra_defconfig | 1 -
arch/arm/firmware/Kconfig | 3 ++-
arch/arm/firmware/trusted_foundations.c | 20 +++++++++++++++++++-
arch/arm/include/asm/trusted_foundations.h | 13 +++++++++----
arch/arm/mach-tegra/cpuidle-tegra114.c | 3 +++
5 files changed, 33 insertions(+), 7 deletions(-)
--
1.8.5.3
^ permalink raw reply
* [PATCH] ARM: imx: correct usecount of IPG, ARM and MMDC clk on i.mx6sl
From: Lucas Stach @ 2014-01-21 10:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390214690-13564-1-git-send-email-b20788@freescale.com>
Am Montag, den 20.01.2014, 18:44 +0800 schrieb Anson Huang:
> IPG, ARM and MMDC's clock should be enabled during kernel boot up,
> so we need to maintain their use count, otherwise, they may be
> disabled unexpectedly if their children's clock are turned off,
> which is not allowed.
>
> Signed-off-by: Anson Huang <b20788@freescale.com>
> ---
> arch/arm/mach-imx/clk-imx6sl.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
> index 78f3bd6..8d720f9 100644
> --- a/arch/arm/mach-imx/clk-imx6sl.c
> +++ b/arch/arm/mach-imx/clk-imx6sl.c
> @@ -291,6 +291,21 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
> pr_warn("%s: failed to set AHB clock rate %d!\n",
> __func__, ret);
>
> + /* Correct usecount of IPG clk */
You don't really correct anything here, so this comment is misleading.
If those are always on clocks, like you suggest in the commit message,
please put this in the comment instead.
Regards,
Lucas
> + ret = clk_prepare_enable(clks[IMX6SL_CLK_IPG]);
> + if (ret)
> + pr_warn("%s: failed to enable IPG clock %d\n", __func__, ret);
> +
> + /* Correct usecount of ARM clk */
> + ret = clk_prepare_enable(clks[IMX6SL_CLK_ARM]);
> + if (ret)
> + pr_warn("%s: failed to enable ARM clock %d\n", __func__, ret);
> +
> + /* Correct usecount of MMDC clk */
> + ret = clk_prepare_enable(clks[IMX6SL_CLK_MMDC_ROOT]);
> + if (ret)
> + pr_warn("%s: failed to enable MMDC clock %d\n", __func__, ret);
> +
> if (IS_ENABLED(CONFIG_USB_MXS_PHY)) {
> clk_prepare_enable(clks[IMX6SL_CLK_USBPHY1_GATE]);
> clk_prepare_enable(clks[IMX6SL_CLK_USBPHY2_GATE]);
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v2 14/15] watchdog: orion: Allow to build on any Orion platform
From: Ezequiel Garcia @ 2014-01-21 10:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121094501.GA13396@lunn.ch>
Hi Andrew, Sebastian:
On Tue, Jan 21, 2014 at 10:45:01AM +0100, Andrew Lunn wrote:
> On Tue, Jan 21, 2014 at 10:41:17AM +0100, Sebastian Hesselbarth wrote:
> > On 01/21/14 10:12, Ezequiel Garcia wrote:
> > >After getting rid of all the mach-specific code, it's now possible
> > >to allow builds in any Orion platform.
> > >
> > >Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > >---
> > > drivers/watchdog/Kconfig | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > >index 5be6e91..1689f72 100644
> > >--- a/drivers/watchdog/Kconfig
> > >+++ b/drivers/watchdog/Kconfig
> > >@@ -282,7 +282,7 @@ config DAVINCI_WATCHDOG
> > >
> > > config ORION_WATCHDOG
> > > tristate "Orion watchdog"
> > >- depends on ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE
> > >+ depends on PLAT_ORION
[..]
> >
> > while above is true now, I tend to rather have ARCH_MVEBU added here.
> > We really want to get rid of both !ARCH_MVEBU and PLAT_ORION and this
> > unnecessarily will make it more complicated.
> >
> > I haven't checked why ARCH_MVEBU at all added PLAT_ORION as dependency,
> > but IIRC it was just because of a missing mbus driver? If it is just
> > this, we should also remove PLAT_ORION from ARCH_MVEBU to have a clean
> > cut between new common arch and existing sub-archs.
>
Hm... not sure why we depend on PLAT_ORION. I'll take a look.
> I took a look at this when moving DT kirkwood into mach-mvebu. There
> is nothing in plat-orion which ARCH_MVEBU needs. So i agree with
> Sebastian, it should be PLAT_ORION || ARCH_MVEBU.
>
OK, will do.
Thanks!
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH REPOST 1/5] ARM: kvm: replace push and pop with stdmb and ldmia instrs to enable assembler.h inclusion
From: Marc Zyngier @ 2014-01-21 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121011825.GI13432@cbox>
On 21/01/14 01:18, Christoffer Dall wrote:
> On Fri, Dec 20, 2013 at 08:48:41AM -0800, Victor Kamensky wrote:
>> Before fix kvm interrupt.S and interrupt_head.S used push and pop assembler
>> instruction. It causes problem if <asm/assembler.h> file should be include. In
>> assembler.h "push" is defined as macro so it causes compilation errors like
>> this:
>
> "Before fix kvm..." doesn't read very pleasently, consider using
> something like "Prior to this commit...."
>
> "causes a problem" or "causes problems"
>
> change "if <asm/assembler.h> file should be include..." to "if
> <asm/assembler.h> is included, because assember.h defines 'push' as a
> macro..."
>
>
>
>>
>> arch/arm/kvm/interrupts.S: Assembler messages:
>> arch/arm/kvm/interrupts.S:51: Error: ARM register expected -- `lsr {r2,r3}'
>>
>> Solution implemented by this patch replaces all 'push {...}' with
>> 'stdmb sp!, {...}' instruction; and all 'pop {...}' with 'ldmia sp!, {...}'.
>>
>> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
>> ---
>> arch/arm/kvm/interrupts.S | 38 +++++++++++++++++++-------------------
>> arch/arm/kvm/interrupts_head.S | 34 +++++++++++++++++-----------------
>> 2 files changed, 36 insertions(+), 36 deletions(-)
>>
>> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
>> index ddc1553..df19133 100644
>> --- a/arch/arm/kvm/interrupts.S
>> +++ b/arch/arm/kvm/interrupts.S
>> @@ -47,7 +47,7 @@ __kvm_hyp_code_start:
>> * instead, ignoring the ipa value.
>> */
>> ENTRY(__kvm_tlb_flush_vmid_ipa)
>> - push {r2, r3}
>> + stmdb sp!, {r2, r3}
>>
>> dsb ishst
>> add r0, r0, #KVM_VTTBR
>> @@ -62,7 +62,7 @@ ENTRY(__kvm_tlb_flush_vmid_ipa)
>> mcrr p15, 6, r2, r3, c2 @ Back to VMID #0
>> isb @ Not necessary if followed by eret
>>
>> - pop {r2, r3}
>> + ldmia sp!, {r2, r3}
>> bx lr
>> ENDPROC(__kvm_tlb_flush_vmid_ipa)
>>
>> @@ -110,7 +110,7 @@ ENTRY(__kvm_vcpu_run)
>> #ifdef CONFIG_VFPv3
>> @ Set FPEXC_EN so the guest doesn't trap floating point instructions
>> VFPFMRX r2, FPEXC @ VMRS
>> - push {r2}
>> + stmdb sp!, {r2}
>> orr r2, r2, #FPEXC_EN
>> VFPFMXR FPEXC, r2 @ VMSR
>> #endif
>> @@ -175,7 +175,7 @@ __kvm_vcpu_return:
>>
>> after_vfp_restore:
>> @ Restore FPEXC_EN which we clobbered on entry
>> - pop {r2}
>> + ldmia sp!, {r2}
>> VFPFMXR FPEXC, r2
>> #endif
>>
>> @@ -260,7 +260,7 @@ ENTRY(kvm_call_hyp)
>>
>> /* Handle undef, svc, pabt, or dabt by crashing with a user notice */
>> .macro bad_exception exception_code, panic_str
>> - push {r0-r2}
>> + stmdb sp!, {r0-r2}
>> mrrc p15, 6, r0, r1, c2 @ Read VTTBR
>> lsr r1, r1, #16
>> ands r1, r1, #0xff
>> @@ -338,7 +338,7 @@ hyp_hvc:
>> * Getting here is either becuase of a trap from a guest or from calling
>> * HVC from the host kernel, which means "switch to Hyp mode".
>> */
>> - push {r0, r1, r2}
>> + stmdb sp!, {r0, r1, r2}
>>
>> @ Check syndrome register
>> mrc p15, 4, r1, c5, c2, 0 @ HSR
>> @@ -361,11 +361,11 @@ hyp_hvc:
>> bne guest_trap @ Guest called HVC
>>
>> host_switch_to_hyp:
>> - pop {r0, r1, r2}
>> + ldmia sp!, {r0, r1, r2}
>>
>> - push {lr}
>> + stmdb sp!, {lr}
>> mrs lr, SPSR
>> - push {lr}
>> + stmdb sp!, {lr}
>>
>> mov lr, r0
>> mov r0, r1
>> @@ -375,9 +375,9 @@ host_switch_to_hyp:
>> THUMB( orr lr, #1)
>> blx lr @ Call the HYP function
>>
>> - pop {lr}
>> + ldmia sp!, {lr}
>> msr SPSR_csxf, lr
>> - pop {lr}
>> + ldmia sp!, {lr}
>> eret
>>
>> guest_trap:
>> @@ -418,7 +418,7 @@ guest_trap:
>>
>> /* Preserve PAR */
>> mrrc p15, 0, r0, r1, c7 @ PAR
>> - push {r0, r1}
>> + stmdb sp!, {r0, r1}
>>
>> /* Resolve IPA using the xFAR */
>> mcr p15, 0, r2, c7, c8, 0 @ ATS1CPR
>> @@ -431,7 +431,7 @@ guest_trap:
>> orr r2, r2, r1, lsl #24
>>
>> /* Restore PAR */
>> - pop {r0, r1}
>> + ldmia sp!, {r0, r1}
>> mcrr p15, 0, r0, r1, c7 @ PAR
>>
>> 3: load_vcpu @ Load VCPU pointer to r0
>> @@ -440,10 +440,10 @@ guest_trap:
>> 1: mov r1, #ARM_EXCEPTION_HVC
>> b __kvm_vcpu_return
>>
>> -4: pop {r0, r1} @ Failed translation, return to guest
>> +4: ldmia sp!, {r0, r1} @ Failed translation, return to guest
>> mcrr p15, 0, r0, r1, c7 @ PAR
>> clrex
>> - pop {r0, r1, r2}
>> + ldmia sp!, {r0, r1, r2}
>> eret
>>
>> /*
>> @@ -455,7 +455,7 @@ guest_trap:
>> #ifdef CONFIG_VFPv3
>> switch_to_guest_vfp:
>> load_vcpu @ Load VCPU pointer to r0
>> - push {r3-r7}
>> + stmdb sp!, {r3-r7}
>>
>> @ NEON/VFP used. Turn on VFP access.
>> set_hcptr vmexit, (HCPTR_TCP(10) | HCPTR_TCP(11))
>> @@ -467,15 +467,15 @@ switch_to_guest_vfp:
>> add r7, r0, #VCPU_VFP_GUEST
>> restore_vfp_state r7
>>
>> - pop {r3-r7}
>> - pop {r0-r2}
>> + ldmia sp!, {r3-r7}
>> + ldmia sp!, {r0-r2}
>> clrex
>> eret
>> #endif
>>
>> .align
>> hyp_irq:
>> - push {r0, r1, r2}
>> + stmdb sp!, {r0, r1, r2}
>> mov r1, #ARM_EXCEPTION_IRQ
>> load_vcpu @ Load VCPU pointer to r0
>> b __kvm_vcpu_return
>> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
>> index 6f18695..c371db7 100644
>> --- a/arch/arm/kvm/interrupts_head.S
>> +++ b/arch/arm/kvm/interrupts_head.S
>> @@ -63,7 +63,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mrs r2, SP_\mode
>> mrs r3, LR_\mode
>> mrs r4, SPSR_\mode
>> - push {r2, r3, r4}
>> + stmdb sp!, {r2, r3, r4}
>> .endm
>>
>> /*
>> @@ -73,13 +73,13 @@ vcpu .req r0 @ vcpu pointer always in r0
>> .macro save_host_regs
>> /* Hyp regs. Only ELR_hyp (SPSR_hyp already saved) */
>> mrs r2, ELR_hyp
>> - push {r2}
>> + stmdb sp!, {r2}
>>
>> /* usr regs */
>> - push {r4-r12} @ r0-r3 are always clobbered
>> + stmdb sp!, {r4-r12} @ r0-r3 are always clobbered
>> mrs r2, SP_usr
>> mov r3, lr
>> - push {r2, r3}
>> + stmdb sp!, {r2, r3}
>>
>> push_host_regs_mode svc
>> push_host_regs_mode abt
>> @@ -95,11 +95,11 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mrs r7, SP_fiq
>> mrs r8, LR_fiq
>> mrs r9, SPSR_fiq
>> - push {r2-r9}
>> + stmdb sp!, {r2-r9}
>> .endm
>>
>> .macro pop_host_regs_mode mode
>> - pop {r2, r3, r4}
>> + ldmia sp!, {r2, r3, r4}
>> msr SP_\mode, r2
>> msr LR_\mode, r3
>> msr SPSR_\mode, r4
>> @@ -110,7 +110,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> * Clobbers all registers, in all modes, except r0 and r1.
>> */
>> .macro restore_host_regs
>> - pop {r2-r9}
>> + ldmia sp!, {r2-r9}
>> msr r8_fiq, r2
>> msr r9_fiq, r3
>> msr r10_fiq, r4
>> @@ -125,12 +125,12 @@ vcpu .req r0 @ vcpu pointer always in r0
>> pop_host_regs_mode abt
>> pop_host_regs_mode svc
>>
>> - pop {r2, r3}
>> + ldmia sp!, {r2, r3}
>> msr SP_usr, r2
>> mov lr, r3
>> - pop {r4-r12}
>> + ldmia sp!, {r4-r12}
>>
>> - pop {r2}
>> + ldmia sp!, {r2}
>> msr ELR_hyp, r2
>> .endm
>>
>> @@ -218,7 +218,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> add r2, vcpu, #VCPU_USR_REG(3)
>> stm r2, {r3-r12}
>> add r2, vcpu, #VCPU_USR_REG(0)
>> - pop {r3, r4, r5} @ r0, r1, r2
>> + ldmia sp!, {r3, r4, r5} @ r0, r1, r2
>> stm r2, {r3, r4, r5}
>> mrs r2, SP_usr
>> mov r3, lr
>> @@ -258,7 +258,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mrc p15, 2, r12, c0, c0, 0 @ CSSELR
>>
>> .if \store_to_vcpu == 0
>> - push {r2-r12} @ Push CP15 registers
>> + stmdb sp!, {r2-r12} @ Push CP15 registers
>> .else
>> str r2, [vcpu, #CP15_OFFSET(c1_SCTLR)]
>> str r3, [vcpu, #CP15_OFFSET(c1_CPACR)]
>> @@ -286,7 +286,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mrc p15, 0, r12, c12, c0, 0 @ VBAR
>>
>> .if \store_to_vcpu == 0
>> - push {r2-r12} @ Push CP15 registers
>> + stmdb sp!, {r2-r12} @ Push CP15 registers
>> .else
>> str r2, [vcpu, #CP15_OFFSET(c13_CID)]
>> str r3, [vcpu, #CP15_OFFSET(c13_TID_URW)]
>> @@ -305,7 +305,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mrrc p15, 0, r4, r5, c7 @ PAR
>>
>> .if \store_to_vcpu == 0
>> - push {r2,r4-r5}
>> + stmdb sp!, {r2,r4-r5}
>> .else
>> str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
>> add r12, vcpu, #CP15_OFFSET(c7_PAR)
>> @@ -322,7 +322,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> */
>> .macro write_cp15_state read_from_vcpu
>> .if \read_from_vcpu == 0
>> - pop {r2,r4-r5}
>> + ldmia sp!, {r2,r4-r5}
>> .else
>> ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
>> add r12, vcpu, #CP15_OFFSET(c7_PAR)
>> @@ -333,7 +333,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mcrr p15, 0, r4, r5, c7 @ PAR
>>
>> .if \read_from_vcpu == 0
>> - pop {r2-r12}
>> + ldmia sp!, {r2-r12}
>> .else
>> ldr r2, [vcpu, #CP15_OFFSET(c13_CID)]
>> ldr r3, [vcpu, #CP15_OFFSET(c13_TID_URW)]
>> @@ -361,7 +361,7 @@ vcpu .req r0 @ vcpu pointer always in r0
>> mcr p15, 0, r12, c12, c0, 0 @ VBAR
>>
>> .if \read_from_vcpu == 0
>> - pop {r2-r12}
>> + ldmia sp!, {r2-r12}
>> .else
>> ldr r2, [vcpu, #CP15_OFFSET(c1_SCTLR)]
>> ldr r3, [vcpu, #CP15_OFFSET(c1_CPACR)]
>> --
>> 1.8.1.4
>>
>
> If you fix to address Dave's comments, then the code change otherwise
> looks good.
How about trying this alternative approach:
It looks like all the users of the push/pop macros are located in
arch/arm/lib (mostly checksumming code). Can't we move these macros to a
separate include file and leave the code that uses push/pop (as defined
by the assembler) alone?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2 02/15] clocksource: orion: Use atomic access for shared registers
From: Sebastian Hesselbarth @ 2014-01-21 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5118193.GjKusj569J@wuerfel>
On 01/21/14 10:46, Arnd Bergmann wrote:
> On Tuesday 21 January 2014 06:12:28 Ezequiel Garcia wrote:
>> -/*
>> - * Thread-safe access to TIMER_CTRL register
>> - * (shared with watchdog timer)
>> - */
>> -void orion_timer_ctrl_clrset(u32 clr, u32 set)
>> -{
>> - spin_lock(&timer_ctrl_lock);
>> - writel((readl(timer_base + TIMER_CTRL) & ~clr) | set,
>> - timer_base + TIMER_CTRL);
>> - spin_unlock(&timer_ctrl_lock);
>> -}
>> -EXPORT_SYMBOL(orion_timer_ctrl_clrset);
>
> I don't understand what's wrong with this function, it seems like
> a cleaner approach than touching the register directly from two
> different drivers. Is this something that would only work on
> orion but not on armadaxp?
The real problem with this is that it resides in orion-time.c which
is fine for Orion SoCs. Armada 370/XP use a different timer and
therefore the _common_ watchdog driver cannot call this function.
Moreover, Dove (out of Orion) and Armada 370/XP will happily live in
one V7 kernel with both time-orion and time-armada-370-xp compiled in.
The idea of the atomic readl/writel was to have something lightweight
and _early_ to allow such locking even for timers.
IIRC, there was some kind of consensus that it is okay to have atomic
readl/writel here.
Sebastian
^ permalink raw reply
* [PATCH v2 02/15] clocksource: orion: Use atomic access for shared registers
From: Daniel Lezcano @ 2014-01-21 9:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-3-git-send-email-ezequiel.garcia@free-electrons.com>
On 01/21/2014 10:12 AM, Ezequiel Garcia wrote:
> Replace the driver-specific thread-safe shared register API
> by the recently introduced atomic_io_clear_set().
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Hi Ezequiel,
in the future, put me in Cc when modifying files in clocksource that
will make my life easier to track the changes.
Thanks.
-- Daniel
> ---
> drivers/clocksource/time-orion.c | 28 ++++++++++------------------
> 1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/clocksource/time-orion.c b/drivers/clocksource/time-orion.c
> index 9c7f018..3f14e56 100644
> --- a/drivers/clocksource/time-orion.c
> +++ b/drivers/clocksource/time-orion.c
> @@ -35,20 +35,6 @@
> #define ORION_ONESHOT_MAX 0xfffffffe
>
> static void __iomem *timer_base;
> -static DEFINE_SPINLOCK(timer_ctrl_lock);
> -
> -/*
> - * Thread-safe access to TIMER_CTRL register
> - * (shared with watchdog timer)
> - */
> -void orion_timer_ctrl_clrset(u32 clr, u32 set)
> -{
> - spin_lock(&timer_ctrl_lock);
> - writel((readl(timer_base + TIMER_CTRL) & ~clr) | set,
> - timer_base + TIMER_CTRL);
> - spin_unlock(&timer_ctrl_lock);
> -}
> -EXPORT_SYMBOL(orion_timer_ctrl_clrset);
> /*
> * Free-running clocksource handling.
> @@ -68,7 +54,8 @@ static int orion_clkevt_next_event(unsigned long delta,
> {
> /* setup and enable one-shot timer */
> writel(delta, timer_base + TIMER1_VAL);
> - orion_timer_ctrl_clrset(TIMER1_RELOAD_EN, TIMER1_EN);
> + atomic_io_modify(timer_base + TIMER_CTRL,
> + TIMER1_RELOAD_EN | TIMER1_EN, TIMER1_EN);
I am not convinced this change is worth.
Could you elaborate the race the spinlock should prevent ?
>
> return 0;
> }
> @@ -80,10 +67,13 @@ static void orion_clkevt_mode(enum clock_event_mode mode,
> /* setup and enable periodic timer at 1/HZ intervals */
> writel(ticks_per_jiffy - 1, timer_base + TIMER1_RELOAD);
> writel(ticks_per_jiffy - 1, timer_base + TIMER1_VAL);
> - orion_timer_ctrl_clrset(0, TIMER1_RELOAD_EN | TIMER1_EN);
> + atomic_io_modify(timer_base + TIMER_CTRL,
> + TIMER1_RELOAD_EN | TIMER1_EN,
> + TIMER1_RELOAD_EN | TIMER1_EN);
> } else {
> /* disable timer */
> - orion_timer_ctrl_clrset(TIMER1_RELOAD_EN | TIMER1_EN, 0);
> + atomic_io_modify(timer_base + TIMER_CTRL,
> + TIMER1_RELOAD_EN | TIMER1_EN, 0);
> }
> }
>
> @@ -131,7 +121,9 @@ static void __init orion_timer_init(struct device_node *np)
> /* setup timer0 as free-running clocksource */
> writel(~0, timer_base + TIMER0_VAL);
> writel(~0, timer_base + TIMER0_RELOAD);
> - orion_timer_ctrl_clrset(0, TIMER0_RELOAD_EN | TIMER0_EN);
> + atomic_io_modify(timer_base + TIMER_CTRL,
> + TIMER0_RELOAD_EN | TIMER0_EN,
> + TIMER0_RELOAD_EN | TIMER0_EN);
> clocksource_mmio_init(timer_base + TIMER0_VAL, "orion_clocksource",
> clk_get_rate(clk), 300, 32,
> clocksource_mmio_readl_down);
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH v2 03/15] watchdog: orion: Use atomic access for shared registers
From: Arnd Bergmann @ 2014-01-21 9:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-4-git-send-email-ezequiel.garcia@free-electrons.com>
On Tuesday 21 January 2014 06:12:29 Ezequiel Garcia wrote:
> writel(~WDT_INT_REQ, BRIDGE_CAUSE);
>
> /* Enable watchdog timer */
> - reg = readl(wdt_reg + TIMER_CTRL);
> - reg |= WDT_EN;
> - writel(reg, wdt_reg + TIMER_CTRL);
> + atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, WDT_EN);
As mentioned in my comment for patch 2, it seems that the exported
orion_timer_ctrl_clrset() function would be more appropriate for this.
> /* Enable reset on watchdog */
> - reg = readl(RSTOUTn_MASK);
> - reg |= WDT_RESET_OUT_EN;
> - writel(reg, RSTOUTn_MASK);
> + atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN);
And this register already has an abstraction in
arch/arm/mach-mvebu/system-controller.c. I would prefer to find a way
to generalize that. I can see multiple ways to do that:
* Turn the "marvell,armada-370-xp-system-controller" device into a "syscon"
device that can be used through regmap from other parts of the kernel.
* Move arch/arm/mach-mvebu/system-controller.c into drivers/soc (we don't
have this directory yet, but have discussed it in the past) to export
soc-specific functions from there.
* Move all of the system-controller implementation into the wdt driver
and require that this driver be enabled in order to do regular system
reset. The system-controller can register itself as a reboot hook,
so you can remove the ".restart = mvebu_restart" part from mach-mvebu.
Arnd
^ permalink raw reply
* [PATCH v2 02/15] clocksource: orion: Use atomic access for shared registers
From: Arnd Bergmann @ 2014-01-21 9:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-3-git-send-email-ezequiel.garcia@free-electrons.com>
On Tuesday 21 January 2014 06:12:28 Ezequiel Garcia wrote:
> -/*
> - * Thread-safe access to TIMER_CTRL register
> - * (shared with watchdog timer)
> - */
> -void orion_timer_ctrl_clrset(u32 clr, u32 set)
> -{
> - spin_lock(&timer_ctrl_lock);
> - writel((readl(timer_base + TIMER_CTRL) & ~clr) | set,
> - timer_base + TIMER_CTRL);
> - spin_unlock(&timer_ctrl_lock);
> -}
> -EXPORT_SYMBOL(orion_timer_ctrl_clrset);
I don't understand what's wrong with this function, it seems like
a cleaner approach than touching the register directly from two
different drivers. Is this something that would only work on
orion but not on armadaxp?
Arnd
^ permalink raw reply
* [PATCH v2 01/15] ARM: Introduce atomic MMIO modify
From: Arnd Bergmann @ 2014-01-21 9:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-2-git-send-email-ezequiel.garcia@free-electrons.com>
On Tuesday 21 January 2014 06:12:27 Ezequiel Garcia wrote:
> Some SoC have MMIO regions that are shared across orthogonal
> subsystems. This commit implements a possible solution for the
> thread-safe access of such regions through a spinlock-protected API.
>
> Concurrent access is protected with a single spinlock for the
> entire MMIO address space. While this protects shared-registers,
> it also serializes access to unrelated/unshared registers.
>
> We add relaxed and non-relaxed variants, by using writel_relaxed and writel,
> respectively. The rationale for this is that some users may not require
> register write completion but only thread-safe access to a register.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
You add the new atomic mmio interfaces in an ARM global header file,
but at the same time make them ARM-only. I'm not opposed to having
interfaces like that, but I'm not convinced they are actually needed
for this case and if we go there, it needs to be done more carefully
and should be available for all architectures so that portable drivers
can use them.
It also seems to duplicate functionality that is already present in
regmap-mmio.
Arnd
^ permalink raw reply
* [PATCH v2 14/15] watchdog: orion: Allow to build on any Orion platform
From: Andrew Lunn @ 2014-01-21 9:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DE40BD.10700@gmail.com>
On Tue, Jan 21, 2014 at 10:41:17AM +0100, Sebastian Hesselbarth wrote:
> On 01/21/14 10:12, Ezequiel Garcia wrote:
> >After getting rid of all the mach-specific code, it's now possible
> >to allow builds in any Orion platform.
> >
> >Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >---
> > drivers/watchdog/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >index 5be6e91..1689f72 100644
> >--- a/drivers/watchdog/Kconfig
> >+++ b/drivers/watchdog/Kconfig
> >@@ -282,7 +282,7 @@ config DAVINCI_WATCHDOG
> >
> > config ORION_WATCHDOG
> > tristate "Orion watchdog"
> >- depends on ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE
> >+ depends on PLAT_ORION
>
> Ezequiel,
>
> while above is true now, I tend to rather have ARCH_MVEBU added here.
> We really want to get rid of both !ARCH_MVEBU and PLAT_ORION and this
> unnecessarily will make it more complicated.
>
> I haven't checked why ARCH_MVEBU at all added PLAT_ORION as dependency,
> but IIRC it was just because of a missing mbus driver? If it is just
> this, we should also remove PLAT_ORION from ARCH_MVEBU to have a clean
> cut between new common arch and existing sub-archs.
I took a look at this when moving DT kirkwood into mach-mvebu. There
is nothing in plat-orion which ARCH_MVEBU needs. So i agree with
Sebastian, it should be PLAT_ORION || ARCH_MVEBU.
Andrew
^ permalink raw reply
* [PATCH v2 09/15] watchdog: orion: Add per-compatible clock initialization
From: Ezequiel Garcia @ 2014-01-21 9:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5BDS8Dh1PR6SJv1CK5Pkq5mub-Hs7c4RhxUn7o5efnbKg@mail.gmail.com>
On Tue, Jan 21, 2014 at 07:18:44AM -0200, Fabio Estevam wrote:
> On Tue, Jan 21, 2014 at 7:12 AM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
>
> > +static int orion_wdt_clock_init(struct platform_device *pdev,
> > + struct orion_watchdog *dev)
> > +{
> > + dev->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(dev->clk))
> > + return -ENODEV;
>
> Shouldn't it be 'return PTR_ERR(dev->clk)' instead?
>
>
[..]
> > + clk_prepare_enable(dev->clk);
>
> clk_prepare_enable may fail, so better check its return value.
>
Indeed.
Thanks!
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 03/15] watchdog: orion: Use atomic access for shared registers
From: Ezequiel Garcia @ 2014-01-21 9:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5BKjpkjQGOOAQ5fwmxhg1SYTmTq4Y9tUCZaDSQiosGYyw@mail.gmail.com>
On Tue, Jan 21, 2014 at 07:19:52AM -0200, Fabio Estevam wrote:
> On Tue, Jan 21, 2014 at 7:12 AM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Since the timer control register is shared with the clocksource driver,
> > use the recently introduced atomic_io_clear_set() to access such register.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> > drivers/watchdog/orion_wdt.c | 20 ++++----------------
> > 1 file changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> > index f7722a4..cf64510 100644
> > --- a/drivers/watchdog/orion_wdt.c
> > +++ b/drivers/watchdog/orion_wdt.c
> > @@ -61,8 +61,6 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
> >
> > static int orion_wdt_start(struct watchdog_device *wdt_dev)
> > {
> > - u32 reg;
> > -
> > spin_lock(&wdt_lock);
>
> Shouldn't this spin_lock be dropped now?
>
Hm... yes. The watchdog core uses a mutex to serialize all the watchdog hooks:
start, stop, ping, set_timeout, etc. So it seems you're right, the spinlock
should be dropped entirely as I see no need for it.
Thanks for the feedback,
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 14/15] watchdog: orion: Allow to build on any Orion platform
From: Sebastian Hesselbarth @ 2014-01-21 9:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-15-git-send-email-ezequiel.garcia@free-electrons.com>
On 01/21/14 10:12, Ezequiel Garcia wrote:
> After getting rid of all the mach-specific code, it's now possible
> to allow builds in any Orion platform.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> drivers/watchdog/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 5be6e91..1689f72 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -282,7 +282,7 @@ config DAVINCI_WATCHDOG
>
> config ORION_WATCHDOG
> tristate "Orion watchdog"
> - depends on ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE
> + depends on PLAT_ORION
Ezequiel,
while above is true now, I tend to rather have ARCH_MVEBU added here.
We really want to get rid of both !ARCH_MVEBU and PLAT_ORION and this
unnecessarily will make it more complicated.
I haven't checked why ARCH_MVEBU at all added PLAT_ORION as dependency,
but IIRC it was just because of a missing mbus driver? If it is just
this, we should also remove PLAT_ORION from ARCH_MVEBU to have a clean
cut between new common arch and existing sub-archs.
Sebastian
> select WATCHDOG_CORE
> help
> Say Y here if to include support for the watchdog timer
>
^ permalink raw reply
* [PATCH RFC 4/6] net: rfkill: gpio: add device tree support
From: Linus Walleij @ 2014-01-21 9:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAVeFuLP4MkfXGG1FMauvUw_J63zRXdhGwxqYy5W98L1wCQNbw@mail.gmail.com>
On Tue, Jan 21, 2014 at 4:11 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sat, Jan 18, 2014 at 8:11 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
>> gpio = devm_gpiod_get_index(&pdev->dev, NULL, 1);
>>
>> Heikki, are you OK with this change?
>>
>> I think this is actually necessary if the ACPI and DT unification
>> pipe dream shall limp forward, we cannot have arguments passed
>> that have a semantic effect on DT but not on ACPI... Drivers
>> that are supposed to use both ACPI and DT will always
>> have to pass NULL as con ID.
>
> I agree that's how it should be be done with the current API if your
> driver can obtain GPIOs from both ACPI and DT. This is a potential
> issue, as drivers are not supposed to make assumptions about who is
> going to be their GPIO provider. Let's say you started a driver with
> only DT in mind, and used gpio_get(dev, con_id) to get your GPIOs. DT
> bindings are thus of the form "con_id-gpio = <phandle>", and set in
> stone. Then later, someone wants to use your driver with ACPI. How do
> you handle that gracefully?
Short answer is you can't. You have to pour backward-compatibility
code into the driver first checking for that property and then falling
back to the new binding if it doesn't exist.
> I'm starting to wonder, now that ACPI is a first-class GPIO provider,
> whether we should not start to encourage the deprecation of the
> "con_id-gpio = <phandle>" binding form in DT and only use a single
> indexed GPIO property per device.
You have a valid point.
> The con_id parameter would then only
> be used as a label, which would also have the nice side-effect that
> all GPIOs used for a given function will be reported under the same
> name no matter what the GPIO provider is.
As discussed earlier in this thread I'm not sure the con_id is
suitable for labelling GPIOs. It'd be better to have a proper name
specified in DT/ACPI instead.
> From an aesthetic point of view, I definitely prefer using con_id to
> identify GPIOs instead of indexes, but I don't see how we can make it
> play nice with ACPI. Thoughts?
Let's ask the DT maintainers...
I'm a bit sceptic to the whole ACPI-DT-API-should-be-unified
just-one-function-call business, as this was just a very simple example
of what can happen to something as simple as
devm_gpiod_get[_index]().
Yours,
Linus Walleij
^ permalink raw reply
* More GPIO madness on iMX6 - and the crappy ARM port of Linux
From: Linus Walleij @ 2014-01-21 9:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAVeFuJhYv_vKqgyyaw7LOzRvErjf4u=5jJCvpoEgA0ZsMj-1Q@mail.gmail.com>
On Tue, Jan 21, 2014 at 3:55 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sat, Jan 18, 2014 at 7:43 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> 1. Today this OPEN_DRAIN flag is not even passed down to
>> the driver so how could it say anything about it :-( it's a pure gpiolib
>> internal flag. We don't know if the hardware can actually even
>> do open drain, we just assume it can.
>>
>> What it should really do - in the best of worlds - is to check if
>> it can cross-reference the GPIO line to a pin in the pin control
>> subsystem, and if that is possible, then ask the pin if it
>> is supporting open drain and set it. It currently has no such
>> cross-calls, it is just assumed that the configuration is consistent,
>> and the actual pin is set up as open drain. But it would make
>> sense to add more cross-calls here, since GPIO is accepting
>> these flags (OPEN_DRAIN/OPEN_SOURCE).
>
> This would definitely work in the case of pinctrl-backed GPIOs, but
> would not cover all GPIO chips. If we want to cover all cases we
> should give drivers a way to way to report or enforce this capability,
> and make the pinctrl cross-reference one of its implementations where
> it can be done.
It can never be done in all cases, since in some cases the
open drain config is just a property of the line and not controlled
by software at all, and the datasheet just says this line is open
drain.
But we should cover the cases where we deal with pure
SW-controlled configs as far as possible.
>> Alexandre: do you have plans for how to handle a dynamic
>> consumer passing flags to its gpio request in the gpiod API?
>
> Do you mean like passing OPEN_DRAIN or OPEN_SOURCE flags to
> gpiod_get(), similarly to what is done for e.g. gpio_request_one()?
Yes...
> In the case of the gpiod API I would rather see these flags defined in
> the GPIO mapping if possible. For platform data it is already possible
> to specify open drain/open source, for DT this is trivial to add.
I figured so much. But we have a consumer in i2c-core.c doing
this for a valid reason.
> ACPI
> would be more of a problem here, but I'm not sure whether the problem
> is relevant for ACPI GPIOs.
ACPI has an open drain/source flag for some GPIO lines IIRC.
> 1) GPIO drivers' request() function get an extra flags argument that
> is passed by the GPIO core with the flags of the mapping. There we can
> define all the range of properties that gpio_request_one() supported.
> The driver's request() will fail it if cannot satisfy these
> properties. That's where the pinctrl cross-reference would take place.
I think this doesn't need to go all the way down into the driver
actually. We can call to pinctrl in the gpiolib core and
keep the gpiochip API simple. The GPIO driver doesn't even need
to know this.
> 2) All properties accepted by gpio_request_one() can also be passed
> through GPIO mappings. That is, probably platform_data and DT. I don't
> know if we ever get to use open drain GPIOs provided by ACPI, if we do
> then this might be a problem.
I think we need that.
>> Like:
>>
>> bool gpiod_input_always_valid(const struct gpio_desc *desc);
>>
>> And leave it up to the core to look at flags, driver characteristics
>> etc and determine whether the input can be trusted?
>
> I am personally a little bit scared by the number of exported
> functions in the GPIO framework. It is a pretty large number for
> something that is supposed to be simple, so I'd like to avoid adding
> more. :)
I objected already when the OPEN_DRAIN et al were added,
but to no avail...
> How about the following:
>
> 1) GPIOs configured as output without the open drain or open source
> flag either return -EINVAL on gpiod_get_value(), or a cached value
> tracked by gpiolib for consistency (probably the latter).
Make sense.
> 2) GPIOs configured as open drain or open source report the actual
> value read on the pin, like i2c-core needs. This requires that, for
> each GPIO that can be set open drain or open source,
> gpiod_input_always_valid() == true.
Yeah, but as seen from the I2C core, the algorithm there needs
to know if the input is always valid or not, and take different
execution paths depending on that. :-/
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 03/15] watchdog: orion: Use atomic access for shared registers
From: Fabio Estevam @ 2014-01-21 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-4-git-send-email-ezequiel.garcia@free-electrons.com>
On Tue, Jan 21, 2014 at 7:12 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Since the timer control register is shared with the clocksource driver,
> use the recently introduced atomic_io_clear_set() to access such register.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> drivers/watchdog/orion_wdt.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index f7722a4..cf64510 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -61,8 +61,6 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
>
> static int orion_wdt_start(struct watchdog_device *wdt_dev)
> {
> - u32 reg;
> -
> spin_lock(&wdt_lock);
Shouldn't this spin_lock be dropped now?
Regards,
Fabio Estevam
^ permalink raw reply
* [PATCH v2 09/15] watchdog: orion: Add per-compatible clock initialization
From: Fabio Estevam @ 2014-01-21 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-10-git-send-email-ezequiel.garcia@free-electrons.com>
On Tue, Jan 21, 2014 at 7:12 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> +static int orion_wdt_clock_init(struct platform_device *pdev,
> + struct orion_watchdog *dev)
> +{
> + dev->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(dev->clk))
> + return -ENODEV;
Shouldn't it be 'return PTR_ERR(dev->clk)' instead?
> + clk_prepare_enable(dev->clk);
clk_prepare_enable may fail, so better check its return value.
Regards,
Fabio Estevam
^ permalink raw reply
* [PATCH v2 15/15] ARM: mvebu: Enable watchdog support in defconfig
From: Ezequiel Garcia @ 2014-01-21 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-1-git-send-email-ezequiel.garcia@free-electrons.com>
Now that we have proper support for Armada 370/XP watchdog
let's enable it in the defconfig.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/configs/mvebu_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/mvebu_defconfig b/arch/arm/configs/mvebu_defconfig
index 594d706..84ec924 100644
--- a/arch/arm/configs/mvebu_defconfig
+++ b/arch/arm/configs/mvebu_defconfig
@@ -60,6 +60,8 @@ CONFIG_GPIOLIB=y
CONFIG_GPIO_SYSFS=y
CONFIG_THERMAL=y
CONFIG_ARMADA_THERMAL=y
+CONFIG_WATCHDOG=y
+CONFIG_ORION_WATCHDOG=y
CONFIG_USB_SUPPORT=y
CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
--
1.8.1.5
^ permalink raw reply related
* [PATCH v2 14/15] watchdog: orion: Allow to build on any Orion platform
From: Ezequiel Garcia @ 2014-01-21 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390295561-3466-1-git-send-email-ezequiel.garcia@free-electrons.com>
After getting rid of all the mach-specific code, it's now possible
to allow builds in any Orion platform.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 5be6e91..1689f72 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -282,7 +282,7 @@ config DAVINCI_WATCHDOG
config ORION_WATCHDOG
tristate "Orion watchdog"
- depends on ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE
+ depends on PLAT_ORION
select WATCHDOG_CORE
help
Say Y here if to include support for the watchdog timer
--
1.8.1.5
^ permalink raw reply related
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