| Message ID | 1375128719-23642-1-git-send-email-galak@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Jul 29, 2013, at 3:11 PM, Kumar Gala wrote: > Add driver for Qualcomm MSM Hardware Mutex block that exists on newer MSM > SoC (MSM8974, etc). > > CC: Jeffrey Hugo <jhugo@codeaurora.org> > CC: Eric Holmberg <eholmber@codeaurora.org> > Signed-off-by: Kumar Gala <galak@codeaurora.org> > --- [ had old device tree mailing list ] > .../devicetree/bindings/arm/msm/tcsr-mutex.txt | 20 +++ > drivers/hwspinlock/Kconfig | 11 ++ > drivers/hwspinlock/Makefile | 1 + > drivers/hwspinlock/msm_hwspinlock.c | 150 +++++++++++++++++++++ > 4 files changed, 182 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt > create mode 100644 drivers/hwspinlock/msm_hwspinlock.c > > diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt > new file mode 100644 > index 0000000..ddd6889 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt > @@ -0,0 +1,20 @@ > +Qualcomm MSM Hardware Mutex Block: > + > +The hardware block provides mutexes utilized between different processors > +on the SoC as part of the communication protocol used by these processors. > + > +Required properties: > +- compatible: should be "qcom,tcsr-mutex" > +- reg: Should contain registers location and length of mutex registers > +- reg-names: > + "mutex-base" - string to identify mutex registers > +- qcom,num-locks: the number of locks/mutexes supported > + > +Example: > + > + qcom,tcsr-mutex@fd484000 { > + compatible = "qcom,tcsr-mutex"; > + reg = <0xfd484000 0x1000>; > + reg-names = "mutex-base"; > + qcom,num-locks = <32>; > + }; > diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig > index 70637d2..192e939 100644 > --- a/drivers/hwspinlock/Kconfig > +++ b/drivers/hwspinlock/Kconfig > @@ -8,6 +8,17 @@ config HWSPINLOCK > > menu "Hardware Spinlock drivers" > > +config HWSPINLOCK_MSM > + tristate "MSM Hardware Spinlock device" > + depends on ARCH_MSM > + select HWSPINLOCK > + help > + Say y here to support the MSM Hardware Mutex functionality, which > + provides a synchronisation mechanism for the various processors on > + the SoC. > + > + If unsure, say N. > + > config HWSPINLOCK_OMAP > tristate "OMAP Hardware Spinlock device" > depends on ARCH_OMAP4 || SOC_OMAP5 > diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile > index 93eb64b..4074c56 100644 > --- a/drivers/hwspinlock/Makefile > +++ b/drivers/hwspinlock/Makefile > @@ -3,5 +3,6 @@ > # > > obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o > +obj-$(CONFIG_HWSPINLOCK_MSM) += msm_hwspinlock.o > obj-$(CONFIG_HWSPINLOCK_OMAP) += omap_hwspinlock.o > obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o > diff --git a/drivers/hwspinlock/msm_hwspinlock.c b/drivers/hwspinlock/msm_hwspinlock.c > new file mode 100644 > index 0000000..c7d80c5 > --- /dev/null > +++ b/drivers/hwspinlock/msm_hwspinlock.c > @@ -0,0 +1,150 @@ > +/* > + * Copyright (c) 2013, The Linux Foundation. All rights reserved. > + * > + * 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/err.h> > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/hwspinlock.h> > + > +#include <asm/io.h> > + > +#include "hwspinlock_internal.h" > + > +#define SPINLOCK_ID_APPS_PROC 1 > +#define BASE_ID 0 > + > +static int msm_hwspinlock_trylock(struct hwspinlock *lock) > +{ > + void __iomem *lock_addr = lock->priv; > + > + writel_relaxed(SPINLOCK_ID_APPS_PROC, lock_addr); > + smp_mb(); > + return readl_relaxed(lock_addr) == SPINLOCK_ID_APPS_PROC; > +} > + > +static void msm_hwspinlock_unlock(struct hwspinlock *lock) > +{ > + int lock_owner; > + void __iomem *lock_addr = lock->priv; > + > + lock_owner = readl_relaxed(lock_addr); > + if (lock_owner != SPINLOCK_ID_APPS_PROC) { > + pr_err("%s: spinlock not owned by Apps (actual owner is %d)\n", > + __func__, lock_owner); > + } > + > + writel_relaxed(0, lock_addr); > + smp_mb(); > +} > + > +static const struct hwspinlock_ops msm_hwspinlock_ops = { > + .trylock = msm_hwspinlock_trylock, > + .unlock = msm_hwspinlock_unlock, > +}; > + > +static struct of_device_id msm_hwspinlock_of_match[]; > +static int msm_hwspinlock_probe(struct platform_device *pdev) > +{ > + int ret, i, stride; > + u32 num_locks; > + struct hwspinlock_device *bank; > + struct hwspinlock *hwlock; > + struct resource *res; > + void __iomem *iobase; > + struct device_node *node = pdev->dev.of_node; > + const struct of_device_id *match; > + > + match = of_match_device(msm_hwspinlock_of_match, &pdev->dev); > + if (!match) > + return -EINVAL; > + > + ret = of_property_read_u32(node, "qcom,num-locks", &num_locks); > + if (ret || (num_locks == 0)) > + return -ENODEV; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mutex-base"); > + iobase = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(iobase)) > + return PTR_ERR(iobase); > + > + bank = devm_kzalloc(&pdev->dev, > + sizeof(*bank) + num_locks * sizeof(*hwlock), GFP_KERNEL); > + if (!bank) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, bank); > + > + for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++) > + hwlock->priv = iobase + i * stride; > + > + ret = hwspin_lock_register(bank, &pdev->dev, &msm_hwspinlock_ops, > + BASE_ID, num_locks); > + return ret; > +} > + > +static int msm_hwspinlock_remove(struct platform_device *pdev) > +{ > + struct hwspinlock_device *bank = platform_get_drvdata(pdev); > + int ret; > + > + ret = hwspin_lock_unregister(bank); > + if (ret) { > + dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); > + return ret; > + } > + > + return 0; > +} > + > +static struct of_device_id msm_hwspinlock_of_match[] = { > + { > + .compatible = "qcom,tcsr-mutex", > + .data = (void *)0x80, /* register stride */ > + }, > + { }, > +}; > + > +static struct platform_driver msm_hwspinlock_driver = { > + .probe = msm_hwspinlock_probe, > + .remove = msm_hwspinlock_remove, > + .driver = { > + .name = "msm_hwspinlock", > + .owner = THIS_MODULE, > + .of_match_table = msm_hwspinlock_of_match, > + }, > +}; > + > +static int __init msm_hwspinlock_init(void) > +{ > + return platform_driver_register(&msm_hwspinlock_driver); > +} > +/* board init code might need to reserve hwspinlocks for predefined purposes */ > +postcore_initcall(msm_hwspinlock_init); > + > +static void __exit msm_hwspinlock_exit(void) > +{ > + platform_driver_unregister(&msm_hwspinlock_driver); > +} > +module_exit(msm_hwspinlock_exit); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Hardware spinlock driver for MSM"); > +MODULE_AUTHOR("Kumar Gala <galak@codeaurora.org>"); > +MODULE_AUTHOR("Jeffrey Hugo <jhugo@codeaurora.org>"); > +MODULE_AUTHOR("Eric Holmberg <eholmber@codeaurora.org>"); > -- > 1.8.2.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/29, Kumar Gala wrote: > > diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt > > new file mode 100644 > > index 0000000..ddd6889 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt Maybe this should go under a new hwspinlock directory? > > @@ -0,0 +1,20 @@ > > +Qualcomm MSM Hardware Mutex Block: > > + > > +The hardware block provides mutexes utilized between different processors > > +on the SoC as part of the communication protocol used by these processors. > > + > > +Required properties: > > +- compatible: should be "qcom,tcsr-mutex" > > +- reg: Should contain registers location and length of mutex registers > > +- reg-names: > > + "mutex-base" - string to identify mutex registers > > +- qcom,num-locks: the number of locks/mutexes supported > > + > > +Example: > > + > > + qcom,tcsr-mutex@fd484000 { Maybe it should be hw-mutex@fd484000? > > + compatible = "qcom,tcsr-mutex"; > > + reg = <0xfd484000 0x1000>; > > + reg-names = "mutex-base"; > > + qcom,num-locks = <32>; > > + }; > > diff --git a/drivers/hwspinlock/msm_hwspinlock.c b/drivers/hwspinlock/msm_hwspinlock.c > > new file mode 100644 > > index 0000000..c7d80c5 > > --- /dev/null > > +++ b/drivers/hwspinlock/msm_hwspinlock.c > > @@ -0,0 +1,150 @@ > > +/* > > + * Copyright (c) 2013, The Linux Foundation. All rights reserved. > > + * > > + * 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/err.h> > > +#include <linux/kernel.h> > > +#include <linux/slab.h> > > +#include <linux/device.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/of.h> > > +#include <linux/of_address.h> > > +#include <linux/of_device.h> > > +#include <linux/hwspinlock.h> > > + > > +#include <asm/io.h> <linux/io.h> please. > > + > > +#include "hwspinlock_internal.h" > > + > > +#define SPINLOCK_ID_APPS_PROC 1 > > +#define BASE_ID 0 > > + > > +static int msm_hwspinlock_trylock(struct hwspinlock *lock) > > +{ > > + void __iomem *lock_addr = lock->priv; > > + > > + writel_relaxed(SPINLOCK_ID_APPS_PROC, lock_addr); > > + smp_mb(); Are you sure you don't want mb() instead? What is this barrier for? Comment please. > > + return readl_relaxed(lock_addr) == SPINLOCK_ID_APPS_PROC; > > +} > > + > > +static void msm_hwspinlock_unlock(struct hwspinlock *lock) > > +{ > > + int lock_owner; > > + void __iomem *lock_addr = lock->priv; > > + > > + lock_owner = readl_relaxed(lock_addr); > > + if (lock_owner != SPINLOCK_ID_APPS_PROC) { > > + pr_err("%s: spinlock not owned by Apps (actual owner is %d)\n", > > + __func__, lock_owner); > > + } > > + > > + writel_relaxed(0, lock_addr); > > + smp_mb(); Same here. What is smp_mb() for? > > +} > > + > > +static const struct hwspinlock_ops msm_hwspinlock_ops = { > > + .trylock = msm_hwspinlock_trylock, > > + .unlock = msm_hwspinlock_unlock, > > +}; > > + > > +static struct of_device_id msm_hwspinlock_of_match[]; Why not just put the match table here then? Also, can it be const? > > +static int msm_hwspinlock_probe(struct platform_device *pdev) > > +{ > > + int ret, i, stride; > > + u32 num_locks; > > + struct hwspinlock_device *bank; > > + struct hwspinlock *hwlock; > > + struct resource *res; > > + void __iomem *iobase; > > + struct device_node *node = pdev->dev.of_node; > > + const struct of_device_id *match; > > + > > + match = of_match_device(msm_hwspinlock_of_match, &pdev->dev); > > + if (!match) > > + return -EINVAL; > > + > > + ret = of_property_read_u32(node, "qcom,num-locks", &num_locks); > > + if (ret || (num_locks == 0)) Drop useless (). > > + return -ENODEV; > > + > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mutex-base"); > > + iobase = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(iobase)) > > + return PTR_ERR(iobase); > > + > > + bank = devm_kzalloc(&pdev->dev, > > + sizeof(*bank) + num_locks * sizeof(*hwlock), GFP_KERNEL); > > + if (!bank) > > + return -ENOMEM; > > + Style Nit: Maybe we could grow a local variable to get this to be one line. size_t array_size; array_size = num_lock * sizeof(*hwlock); bank = devm_kzalloc(&pdev->dev, sizeof(*bank) + array_size, GFP_KERNEL);
On Jul 29, 2013, at 4:40 PM, Stephen Boyd wrote: > On 07/29, Kumar Gala wrote: >>> diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >>> new file mode 100644 >>> index 0000000..ddd6889 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt > > Maybe this should go under a new hwspinlock directory? Will look for Ohad to comment on this. > >>> @@ -0,0 +1,20 @@ >>> +Qualcomm MSM Hardware Mutex Block: >>> + >>> +The hardware block provides mutexes utilized between different processors >>> +on the SoC as part of the communication protocol used by these processors. >>> + >>> +Required properties: >>> +- compatible: should be "qcom,tcsr-mutex" >>> +- reg: Should contain registers location and length of mutex registers >>> +- reg-names: >>> + "mutex-base" - string to identify mutex registers >>> +- qcom,num-locks: the number of locks/mutexes supported >>> + >>> +Example: >>> + >>> + qcom,tcsr-mutex@fd484000 { > > Maybe it should be hw-mutex@fd484000? again, will look for Ohad to make some comment on this. >>> + compatible = "qcom,tcsr-mutex"; >>> + reg = <0xfd484000 0x1000>; >>> + reg-names = "mutex-base"; >>> + qcom,num-locks = <32>; >>> + }; >>> diff --git a/drivers/hwspinlock/msm_hwspinlock.c b/drivers/hwspinlock/msm_hwspinlock.c >>> new file mode 100644 >>> index 0000000..c7d80c5 >>> --- /dev/null >>> +++ b/drivers/hwspinlock/msm_hwspinlock.c >>> @@ -0,0 +1,150 @@ >>> +/* >>> + * Copyright (c) 2013, The Linux Foundation. All rights reserved. >>> + * >>> + * 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/err.h> >>> +#include <linux/kernel.h> >>> +#include <linux/slab.h> >>> +#include <linux/device.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/of.h> >>> +#include <linux/of_address.h> >>> +#include <linux/of_device.h> >>> +#include <linux/hwspinlock.h> >>> + >>> +#include <asm/io.h> > > <linux/io.h> please. will change, I choice <asm/io.h> since *_relaxed are arm specific. >>> + >>> +#include "hwspinlock_internal.h" >>> + >>> +#define SPINLOCK_ID_APPS_PROC 1 >>> +#define BASE_ID 0 >>> + >>> +static int msm_hwspinlock_trylock(struct hwspinlock *lock) >>> +{ >>> + void __iomem *lock_addr = lock->priv; >>> + >>> + writel_relaxed(SPINLOCK_ID_APPS_PROC, lock_addr); >>> + smp_mb(); > > Are you sure you don't want mb() instead? What is this barrier > for? Comment please. Hopefully, Eric or Jeff can comment. >>> + return readl_relaxed(lock_addr) == SPINLOCK_ID_APPS_PROC; >>> +} >>> + >>> +static void msm_hwspinlock_unlock(struct hwspinlock *lock) >>> +{ >>> + int lock_owner; >>> + void __iomem *lock_addr = lock->priv; >>> + >>> + lock_owner = readl_relaxed(lock_addr); >>> + if (lock_owner != SPINLOCK_ID_APPS_PROC) { >>> + pr_err("%s: spinlock not owned by Apps (actual owner is %d)\n", >>> + __func__, lock_owner); >>> + } >>> + >>> + writel_relaxed(0, lock_addr); >>> + smp_mb(); > > Same here. What is smp_mb() for? Hopefully, Eric or Jeff can comment. > >>> +} >>> + >>> +static const struct hwspinlock_ops msm_hwspinlock_ops = { >>> + .trylock = msm_hwspinlock_trylock, >>> + .unlock = msm_hwspinlock_unlock, >>> +}; >>> + >>> +static struct of_device_id msm_hwspinlock_of_match[]; > > Why not just put the match table here then? Also, can it be > const? Will change and make const > >>> +static int msm_hwspinlock_probe(struct platform_device *pdev) >>> +{ >>> + int ret, i, stride; >>> + u32 num_locks; >>> + struct hwspinlock_device *bank; >>> + struct hwspinlock *hwlock; >>> + struct resource *res; >>> + void __iomem *iobase; >>> + struct device_node *node = pdev->dev.of_node; >>> + const struct of_device_id *match; >>> + >>> + match = of_match_device(msm_hwspinlock_of_match, &pdev->dev); >>> + if (!match) >>> + return -EINVAL; >>> + >>> + ret = of_property_read_u32(node, "qcom,num-locks", &num_locks); >>> + if (ret || (num_locks == 0)) > > Drop useless (). done > >>> + return -ENODEV; >>> + >>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mutex-base"); >>> + iobase = devm_ioremap_resource(&pdev->dev, res); >>> + if (IS_ERR(iobase)) >>> + return PTR_ERR(iobase); >>> + >>> + bank = devm_kzalloc(&pdev->dev, >>> + sizeof(*bank) + num_locks * sizeof(*hwlock), GFP_KERNEL); >>> + if (!bank) >>> + return -ENOMEM; >>> + > > Style Nit: Maybe we could grow a local variable to get this to be > one line. > > size_t array_size; > > array_size = num_lock * sizeof(*hwlock); > bank = devm_kzalloc(&pdev->dev, sizeof(*bank) + array_size, GFP_KERNEL); done - k > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/29, Kumar Gala wrote: > > On Jul 29, 2013, at 4:40 PM, Stephen Boyd wrote: > > > On 07/29, Kumar Gala wrote: > >>> +#include <linux/of_device.h> > >>> +#include <linux/hwspinlock.h> > >>> + > >>> +#include <asm/io.h> > > > > <linux/io.h> please. > > will change, I choice <asm/io.h> since *_relaxed are arm specific. > I thought readl_relaxed() was universal for HAS_IOMEM arches. Unfortunately writel_relaxed() isn't defined on all arches. Luckily this driver depends on ARCH_MSM so it isn't a problem. Perhaps it should just depend on HAS_IOMEM so we can use it on arm64 and/or hexagon in the future without having to change the Kconfig. That would require introducing writel_relaxed() into the other arches that don't have it right now.
On Jul 29, 2013, at 4:54 PM, Kumar Gala wrote: > > On Jul 29, 2013, at 4:40 PM, Stephen Boyd wrote: > >> On 07/29, Kumar Gala wrote: >>>> diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >>>> new file mode 100644 >>>> index 0000000..ddd6889 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >> >> Maybe this should go under a new hwspinlock directory? > > Will look for Ohad to comment on this. > >> >>>> @@ -0,0 +1,20 @@ >>>> +Qualcomm MSM Hardware Mutex Block: >>>> + >>>> +The hardware block provides mutexes utilized between different processors >>>> +on the SoC as part of the communication protocol used by these processors. >>>> + >>>> +Required properties: >>>> +- compatible: should be "qcom,tcsr-mutex" >>>> +- reg: Should contain registers location and length of mutex registers >>>> +- reg-names: >>>> + "mutex-base" - string to identify mutex registers >>>> +- qcom,num-locks: the number of locks/mutexes supported >>>> + >>>> +Example: >>>> + >>>> + qcom,tcsr-mutex@fd484000 { >> >> Maybe it should be hw-mutex@fd484000? > > again, will look for Ohad to make some comment on this. > >>>> + compatible = "qcom,tcsr-mutex"; >>>> + reg = <0xfd484000 0x1000>; >>>> + reg-names = "mutex-base"; >>>> + qcom,num-locks = <32>; >>>> + }; Ohad, ping. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+ Grant On Thu, Aug 1, 2013 at 5:10 PM, Kumar Gala <galak@codeaurora.org> wrote: >> On Jul 29, 2013, at 4:40 PM, Stephen Boyd wrote: >>> On 07/29, Kumar Gala wrote: >>>>> diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >>>>> new file mode 100644 >>>>> index 0000000..ddd6889 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >>> >>> Maybe this should go under a new hwspinlock directory? >> >> Will look for Ohad to comment on this. >> >>> >>>>> @@ -0,0 +1,20 @@ >>>>> +Qualcomm MSM Hardware Mutex Block: >>>>> + >>>>> +The hardware block provides mutexes utilized between different processors >>>>> +on the SoC as part of the communication protocol used by these processors. >>>>> + >>>>> +Required properties: >>>>> +- compatible: should be "qcom,tcsr-mutex" >>>>> +- reg: Should contain registers location and length of mutex registers >>>>> +- reg-names: >>>>> + "mutex-base" - string to identify mutex registers >>>>> +- qcom,num-locks: the number of locks/mutexes supported >>>>> + >>>>> +Example: >>>>> + >>>>> + qcom,tcsr-mutex@fd484000 { >>> >>> Maybe it should be hw-mutex@fd484000? >> >> again, will look for Ohad to make some comment on this. >> >>>>> + compatible = "qcom,tcsr-mutex"; >>>>> + reg = <0xfd484000 0x1000>; >>>>> + reg-names = "mutex-base"; >>>>> + qcom,num-locks = <32>; >>>>> + }; > > Ohad, ping. I'd prefer a DT maintainer to take a look at your two open questions above, especially if I'm to merge a new file into the devicetree Documentation folder. Grant, any chance you have a moment to take a look? Otherwise, Stephen - do we have your Ack here? I was happy to see your review but not sure what's the latest status. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/10/13 12:11, Ohad Ben-Cohen wrote: > Otherwise, Stephen - do we have your Ack here? I was happy to see your > review but not sure what's the latest status. The smp_mb() should be removed. Otherwise I'm willing to accept that we only build this driver on ARCH_MSM for now. We can fix that in the future to be available to hexagon if they ever want it. After that it just needs a DT review/ack so I'll be happy to add a reviewed-by on v3.
On Aug 12, 2013, at 11:30 AM, Stephen Boyd wrote: > On 08/10/13 12:11, Ohad Ben-Cohen wrote: >> Otherwise, Stephen - do we have your Ack here? I was happy to see your >> review but not sure what's the latest status. > > The smp_mb() should be removed. Otherwise I'm willing to accept that we > only build this driver on ARCH_MSM for now. We can fix that in the > future to be available to hexagon if they ever want it. After that it > just needs a DT review/ack so I'll be happy to add a reviewed-by on v3. Yeah, waiting on our DT resolution before posting v3 w/various changes like removing the smp_mb() and a few other things. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 10, 2013, at 2:11 PM, Ohad Ben-Cohen wrote: > + Grant > > On Thu, Aug 1, 2013 at 5:10 PM, Kumar Gala <galak@codeaurora.org> wrote: >>> On Jul 29, 2013, at 4:40 PM, Stephen Boyd wrote: >>>> On 07/29, Kumar Gala wrote: >>>>>> diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >>>>>> new file mode 100644 >>>>>> index 0000000..ddd6889 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt >>>> >>>> Maybe this should go under a new hwspinlock directory? >>> >>> Will look for Ohad to comment on this. >>> >>>> >>>>>> @@ -0,0 +1,20 @@ >>>>>> +Qualcomm MSM Hardware Mutex Block: >>>>>> + >>>>>> +The hardware block provides mutexes utilized between different processors >>>>>> +on the SoC as part of the communication protocol used by these processors. >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible: should be "qcom,tcsr-mutex" >>>>>> +- reg: Should contain registers location and length of mutex registers >>>>>> +- reg-names: >>>>>> + "mutex-base" - string to identify mutex registers >>>>>> +- qcom,num-locks: the number of locks/mutexes supported >>>>>> + >>>>>> +Example: >>>>>> + >>>>>> + qcom,tcsr-mutex@fd484000 { >>>> >>>> Maybe it should be hw-mutex@fd484000? >>> >>> again, will look for Ohad to make some comment on this. >>> >>>>>> + compatible = "qcom,tcsr-mutex"; >>>>>> + reg = <0xfd484000 0x1000>; >>>>>> + reg-names = "mutex-base"; >>>>>> + qcom,num-locks = <32>; >>>>>> + }; >> >> Ohad, ping. > > I'd prefer a DT maintainer to take a look at your two open questions > above, especially if I'm to merge a new file into the devicetree > Documentation folder. > > Grant, any chance you have a moment to take a look? > > Otherwise, Stephen - do we have your Ack here? I was happy to see your > review but not sure what's the latest status. > > Thanks, > Ohad. > -- So I think I'd ask you to recommend a name, should we just us 'hwspinlock'. The general view from ePAPR and dts is the node name should be a bit more generic (like ethernet or pci). So what would you suggest? - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 12, 2013 at 8:24 PM, Kumar Gala <galak@codeaurora.org> wrote:
> So I think I'd ask you to recommend a name, should we just us 'hwspinlock'. The general view from ePAPR and dts is the node name should be a bit more generic (like ethernet or pci). So what would you suggest?
How about 'hwlock' ?
This should be generic enough to cover all instances of hardware locks
(we already know hwspinlock isn't generic enough, as there are some
mutex-like hardware locks which doesn't require busy looping).
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 13, 2013, at 1:01 AM, Ohad Ben-Cohen wrote: > On Mon, Aug 12, 2013 at 8:24 PM, Kumar Gala <galak@codeaurora.org> wrote: >> So I think I'd ask you to recommend a name, should we just us 'hwspinlock'. The general view from ePAPR and dts is the node name should be a bit more generic (like ethernet or pci). So what would you suggest? > > How about 'hwlock' ? > > This should be generic enough to cover all instances of hardware locks > (we already know hwspinlock isn't generic enough, as there are some > mutex-like hardware locks which doesn't require busy looping). hwlock sounds good to me, I'll go with it. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt new file mode 100644 index 0000000..ddd6889 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt @@ -0,0 +1,20 @@ +Qualcomm MSM Hardware Mutex Block: + +The hardware block provides mutexes utilized between different processors +on the SoC as part of the communication protocol used by these processors. + +Required properties: +- compatible: should be "qcom,tcsr-mutex" +- reg: Should contain registers location and length of mutex registers +- reg-names: + "mutex-base" - string to identify mutex registers +- qcom,num-locks: the number of locks/mutexes supported + +Example: + + qcom,tcsr-mutex@fd484000 { + compatible = "qcom,tcsr-mutex"; + reg = <0xfd484000 0x1000>; + reg-names = "mutex-base"; + qcom,num-locks = <32>; + }; diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig index 70637d2..192e939 100644 --- a/drivers/hwspinlock/Kconfig +++ b/drivers/hwspinlock/Kconfig @@ -8,6 +8,17 @@ config HWSPINLOCK menu "Hardware Spinlock drivers" +config HWSPINLOCK_MSM + tristate "MSM Hardware Spinlock device" + depends on ARCH_MSM + select HWSPINLOCK + help + Say y here to support the MSM Hardware Mutex functionality, which + provides a synchronisation mechanism for the various processors on + the SoC. + + If unsure, say N. + config HWSPINLOCK_OMAP tristate "OMAP Hardware Spinlock device" depends on ARCH_OMAP4 || SOC_OMAP5 diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile index 93eb64b..4074c56 100644 --- a/drivers/hwspinlock/Makefile +++ b/drivers/hwspinlock/Makefile @@ -3,5 +3,6 @@ # obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o +obj-$(CONFIG_HWSPINLOCK_MSM) += msm_hwspinlock.o obj-$(CONFIG_HWSPINLOCK_OMAP) += omap_hwspinlock.o obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o diff --git a/drivers/hwspinlock/msm_hwspinlock.c b/drivers/hwspinlock/msm_hwspinlock.c new file mode 100644 index 0000000..c7d80c5 --- /dev/null +++ b/drivers/hwspinlock/msm_hwspinlock.c @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2013, The Linux Foundation. All rights reserved. + * + * 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/err.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/hwspinlock.h> + +#include <asm/io.h> + +#include "hwspinlock_internal.h" + +#define SPINLOCK_ID_APPS_PROC 1 +#define BASE_ID 0 + +static int msm_hwspinlock_trylock(struct hwspinlock *lock) +{ + void __iomem *lock_addr = lock->priv; + + writel_relaxed(SPINLOCK_ID_APPS_PROC, lock_addr); + smp_mb(); + return readl_relaxed(lock_addr) == SPINLOCK_ID_APPS_PROC; +} + +static void msm_hwspinlock_unlock(struct hwspinlock *lock) +{ + int lock_owner; + void __iomem *lock_addr = lock->priv; + + lock_owner = readl_relaxed(lock_addr); + if (lock_owner != SPINLOCK_ID_APPS_PROC) { + pr_err("%s: spinlock not owned by Apps (actual owner is %d)\n", + __func__, lock_owner); + } + + writel_relaxed(0, lock_addr); + smp_mb(); +} + +static const struct hwspinlock_ops msm_hwspinlock_ops = { + .trylock = msm_hwspinlock_trylock, + .unlock = msm_hwspinlock_unlock, +}; + +static struct of_device_id msm_hwspinlock_of_match[]; +static int msm_hwspinlock_probe(struct platform_device *pdev) +{ + int ret, i, stride; + u32 num_locks; + struct hwspinlock_device *bank; + struct hwspinlock *hwlock; + struct resource *res; + void __iomem *iobase; + struct device_node *node = pdev->dev.of_node; + const struct of_device_id *match; + + match = of_match_device(msm_hwspinlock_of_match, &pdev->dev); + if (!match) + return -EINVAL; + + ret = of_property_read_u32(node, "qcom,num-locks", &num_locks); + if (ret || (num_locks == 0)) + return -ENODEV; + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mutex-base"); + iobase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(iobase)) + return PTR_ERR(iobase); + + bank = devm_kzalloc(&pdev->dev, + sizeof(*bank) + num_locks * sizeof(*hwlock), GFP_KERNEL); + if (!bank) + return -ENOMEM; + + platform_set_drvdata(pdev, bank); + + for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++) + hwlock->priv = iobase + i * stride; + + ret = hwspin_lock_register(bank, &pdev->dev, &msm_hwspinlock_ops, + BASE_ID, num_locks); + return ret; +} + +static int msm_hwspinlock_remove(struct platform_device *pdev) +{ + struct hwspinlock_device *bank = platform_get_drvdata(pdev); + int ret; + + ret = hwspin_lock_unregister(bank); + if (ret) { + dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); + return ret; + } + + return 0; +} + +static struct of_device_id msm_hwspinlock_of_match[] = { + { + .compatible = "qcom,tcsr-mutex", + .data = (void *)0x80, /* register stride */ + }, + { }, +}; + +static struct platform_driver msm_hwspinlock_driver = { + .probe = msm_hwspinlock_probe, + .remove = msm_hwspinlock_remove, + .driver = { + .name = "msm_hwspinlock", + .owner = THIS_MODULE, + .of_match_table = msm_hwspinlock_of_match, + }, +}; + +static int __init msm_hwspinlock_init(void) +{ + return platform_driver_register(&msm_hwspinlock_driver); +} +/* board init code might need to reserve hwspinlocks for predefined purposes */ +postcore_initcall(msm_hwspinlock_init); + +static void __exit msm_hwspinlock_exit(void) +{ + platform_driver_unregister(&msm_hwspinlock_driver); +} +module_exit(msm_hwspinlock_exit); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Hardware spinlock driver for MSM"); +MODULE_AUTHOR("Kumar Gala <galak@codeaurora.org>"); +MODULE_AUTHOR("Jeffrey Hugo <jhugo@codeaurora.org>"); +MODULE_AUTHOR("Eric Holmberg <eholmber@codeaurora.org>");
Add driver for Qualcomm MSM Hardware Mutex block that exists on newer MSM SoC (MSM8974, etc). CC: Jeffrey Hugo <jhugo@codeaurora.org> CC: Eric Holmberg <eholmber@codeaurora.org> Signed-off-by: Kumar Gala <galak@codeaurora.org> --- .../devicetree/bindings/arm/msm/tcsr-mutex.txt | 20 +++ drivers/hwspinlock/Kconfig | 11 ++ drivers/hwspinlock/Makefile | 1 + drivers/hwspinlock/msm_hwspinlock.c | 150 +++++++++++++++++++++ 4 files changed, 182 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/tcsr-mutex.txt create mode 100644 drivers/hwspinlock/msm_hwspinlock.c