* [PATCH v6 0/2] ohci and ehci-platform clks, phy and dt support
From: Hans de Goede @ 2014-01-15 16:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
And here is v6 of my ohci and ehci-platform clks, phy and dt support patch-set,
this version addresses the 2 small bugs Alan found.
Other then that there are no changes compared to v5.
Regards,
Hans
^ permalink raw reply
* [PATCH v6 1/2] ohci-platform: Add support for devicetree instantiation
From: Hans de Goede @ 2014-01-15 16:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389802104-17330-1-git-send-email-hdegoede@redhat.com>
Add support for ohci-platform instantiation from devicetree, including
optionally getting clks and a phy from devicetree, and enabling / disabling
those on power_on / off.
This should allow using ohci-platform from devicetree in various cases.
Specifically after this commit it can be used for the ohci controller found
on Allwinner sunxi SoCs.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Documentation/devicetree/bindings/usb/usb-ohci.txt | 22 +++
drivers/usb/host/ohci-platform.c | 164 ++++++++++++++++++---
2 files changed, 162 insertions(+), 24 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/usb-ohci.txt
diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
new file mode 100644
index 0000000..f9d6c73
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -0,0 +1,22 @@
+USB OHCI controllers
+
+Required properties:
+- compatible : "usb-ohci"
+- reg : ohci controller register range (address and length)
+- interrupts : ohci controller interrupt
+
+Optional properties:
+- clocks : a list of phandle + clock specifier pairs
+- phys : phandle + phy specifier pair
+- phy-names : "usb"
+
+Example:
+
+ ohci0: ohci at 0x01c14400 {
+ compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
+ reg = <0x01c14400 0x100>;
+ interrupts = <64>;
+ clocks = <&usb_clk 6>, <&ahb_gates 2>;
+ phys = <&usbphy 1>;
+ phy-names = "usb";
+ };
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index f351ff5..0262b13 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -3,6 +3,7 @@
*
* Copyright 2007 Michael Buesch <m@bues.ch>
* Copyright 2011-2012 Hauke Mehrtens <hauke@hauke-m.de>
+ * Copyright 2014 Hans de Goede <hdegoede@redhat.com>
*
* Derived from the OCHI-SSB driver
* Derived from the OHCI-PCI driver
@@ -14,11 +15,14 @@
* Licensed under the GNU/GPL. See COPYING for details.
*/
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
#include <linux/hrtimer.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/err.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/usb/ohci_pdriver.h>
#include <linux/usb.h>
@@ -27,6 +31,13 @@
#include "ohci.h"
#define DRIVER_DESC "OHCI generic platform driver"
+#define OHCI_MAX_CLKS 3
+#define hcd_to_ohci_priv(h) ((struct ohci_platform_priv *)hcd_to_ohci(h)->priv)
+
+struct ohci_platform_priv {
+ struct clk *clks[OHCI_MAX_CLKS];
+ struct phy *phy;
+};
static const char hcd_name[] = "ohci-platform";
@@ -48,11 +59,67 @@ static int ohci_platform_reset(struct usb_hcd *hcd)
return ohci_setup(hcd);
}
+static int ohci_platform_power_on(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+ int clk, ret;
+
+ for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) {
+ ret = clk_prepare_enable(priv->clks[clk]);
+ if (ret)
+ goto err_disable_clks;
+ }
+
+ if (priv->phy) {
+ ret = phy_init(priv->phy);
+ if (ret)
+ goto err_disable_clks;
+
+ ret = phy_power_on(priv->phy);
+ if (ret)
+ goto err_exit_phy;
+ }
+
+ return 0;
+
+err_exit_phy:
+ phy_exit(priv->phy);
+err_disable_clks:
+ while (--clk >= 0)
+ clk_disable_unprepare(priv->clks[clk]);
+
+ return ret;
+}
+
+static void ohci_platform_power_off(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+ int clk;
+
+ if (priv->phy) {
+ phy_power_off(priv->phy);
+ phy_exit(priv->phy);
+ }
+
+ for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--)
+ if (priv->clks[clk])
+ clk_disable_unprepare(priv->clks[clk]);
+}
+
static struct hc_driver __read_mostly ohci_platform_hc_driver;
static const struct ohci_driver_overrides platform_overrides __initconst = {
- .product_desc = "Generic Platform OHCI controller",
- .reset = ohci_platform_reset,
+ .product_desc = "Generic Platform OHCI controller",
+ .reset = ohci_platform_reset,
+ .extra_priv_size = sizeof(struct ohci_platform_priv),
+};
+
+static struct usb_ohci_pdata ohci_platform_defaults = {
+ .power_on = ohci_platform_power_on,
+ .power_suspend = ohci_platform_power_off,
+ .power_off = ohci_platform_power_off,
};
static int ohci_platform_probe(struct platform_device *dev)
@@ -60,17 +127,23 @@ static int ohci_platform_probe(struct platform_device *dev)
struct usb_hcd *hcd;
struct resource *res_mem;
struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
- int irq;
- int err = -ENOMEM;
-
- if (!pdata) {
- WARN_ON(1);
- return -ENODEV;
- }
+ struct ohci_platform_priv *priv;
+ int err, irq, clk = 0;
if (usb_disabled())
return -ENODEV;
+ /*
+ * Use reasonable defaults so platforms don't have to provide these
+ * with DT probing on ARM.
+ */
+ if (!pdata)
+ pdata = &ohci_platform_defaults;
+
+ err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
+ if (err)
+ return err;
+
irq = platform_get_irq(dev, 0);
if (irq < 0) {
dev_err(&dev->dev, "no irq provided");
@@ -83,17 +156,40 @@ static int ohci_platform_probe(struct platform_device *dev)
return -ENXIO;
}
+ hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev,
+ dev_name(&dev->dev));
+ if (!hcd)
+ return -ENOMEM;
+
+ platform_set_drvdata(dev, hcd);
+ dev->dev.platform_data = pdata;
+ priv = hcd_to_ohci_priv(hcd);
+
+ if (pdata == &ohci_platform_defaults && dev->dev.of_node) {
+ priv->phy = devm_phy_get(&dev->dev, "usb");
+ if (IS_ERR(priv->phy)) {
+ err = PTR_ERR(priv->phy);
+ if (err == -EPROBE_DEFER)
+ goto err_put_hcd;
+ priv->phy = NULL;
+ }
+
+ for (clk = 0; clk < OHCI_MAX_CLKS; clk++) {
+ priv->clks[clk] = of_clk_get(dev->dev.of_node, clk);
+ if (IS_ERR(priv->clks[clk])) {
+ err = PTR_ERR(priv->clks[clk]);
+ if (err == -EPROBE_DEFER)
+ goto err_put_clks;
+ priv->clks[clk] = NULL;
+ break;
+ }
+ }
+ }
+
if (pdata->power_on) {
err = pdata->power_on(dev);
if (err < 0)
- return err;
- }
-
- hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev,
- dev_name(&dev->dev));
- if (!hcd) {
- err = -ENOMEM;
- goto err_power;
+ goto err_put_clks;
}
hcd->rsrc_start = res_mem->start;
@@ -102,21 +198,25 @@ static int ohci_platform_probe(struct platform_device *dev)
hcd->regs = devm_ioremap_resource(&dev->dev, res_mem);
if (IS_ERR(hcd->regs)) {
err = PTR_ERR(hcd->regs);
- goto err_put_hcd;
+ goto err_power;
}
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err)
- goto err_put_hcd;
-
- platform_set_drvdata(dev, hcd);
+ goto err_power;
return err;
-err_put_hcd:
- usb_put_hcd(hcd);
err_power:
if (pdata->power_off)
pdata->power_off(dev);
+err_put_clks:
+ while (--clk >= 0)
+ clk_put(priv->clks[clk]);
+err_put_hcd:
+ if (pdata == &ohci_platform_defaults)
+ dev->dev.platform_data = NULL;
+
+ usb_put_hcd(hcd);
return err;
}
@@ -125,13 +225,22 @@ static int ohci_platform_remove(struct platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
+ struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+ int clk;
usb_remove_hcd(hcd);
- usb_put_hcd(hcd);
if (pdata->power_off)
pdata->power_off(dev);
+ for (clk = 0; priv->clks[clk] && clk < OHCI_MAX_CLKS; clk++)
+ clk_put(priv->clks[clk]);
+
+ usb_put_hcd(hcd);
+
+ if (pdata == &ohci_platform_defaults)
+ dev->dev.platform_data = NULL;
+
return 0;
}
@@ -178,6 +287,12 @@ static int ohci_platform_resume(struct device *dev)
#define ohci_platform_resume NULL
#endif /* CONFIG_PM */
+static const struct of_device_id ohci_platform_ids[] = {
+ { .compatible = "usb-ohci", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ohci_platform_ids);
+
static const struct platform_device_id ohci_platform_table[] = {
{ "ohci-platform", 0 },
{ }
@@ -198,6 +313,7 @@ static struct platform_driver ohci_platform_driver = {
.owner = THIS_MODULE,
.name = "ohci-platform",
.pm = &ohci_platform_pm_ops,
+ .of_match_table = ohci_platform_ids,
}
};
--
1.8.4.2
^ permalink raw reply related
* [PATCH v6 2/2] ehci-platform: Add support for clks and phy passed through devicetree
From: Hans de Goede @ 2014-01-15 16:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389802104-17330-1-git-send-email-hdegoede@redhat.com>
Currently ehci-platform is only used in combination with devicetree when used
with some Via socs. By extending it to (optionally) get clks and a phy from
devicetree, and enabling / disabling those on power_on / off, it can be used
more generically. Specifically after this commit it can be used for the
ehci controller on Allwinner sunxi SoCs.
Since ehci-platform is intended to handle any generic enough non pci ehci
device, add a "usb-ehci" compatibility string.
There already is a usb-ehci device-tree bindings document, update this
with clks and phy bindings info.
Although actually quite generic so far the via,vt8500 compatibilty string
had its own bindings document. Somehow we even ended up with 2 of them. Since
these provide no extra information over the generic usb-ehci documentation,
this patch removes them.
The ehci-ppc-of.c driver also claims the usb-ehci compatibility string,
even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is
not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid
2 drivers claiming the same compatibility string getting build on ppc.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Documentation/devicetree/bindings/usb/usb-ehci.txt | 25 +++-
.../devicetree/bindings/usb/via,vt8500-ehci.txt | 15 ---
.../devicetree/bindings/usb/vt8500-ehci.txt | 12 --
drivers/usb/host/Kconfig | 1 +
drivers/usb/host/ehci-platform.c | 149 +++++++++++++++++----
5 files changed, 142 insertions(+), 60 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt
diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
index fa18612..fad10f3 100644
--- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
@@ -7,13 +7,14 @@ Required properties:
(debug-port or other) can be also specified here, but only after
definition of standard EHCI registers.
- interrupts : one EHCI interrupt should be described here.
-If device registers are implemented in big endian mode, the device
-node should have "big-endian-regs" property.
-If controller implementation operates with big endian descriptors,
-"big-endian-desc" property should be specified.
-If both big endian registers and descriptors are used by the controller
-implementation, "big-endian" property can be specified instead of having
-both "big-endian-regs" and "big-endian-desc".
+
+Optional properties:
+ - big-endian-regs : boolean, set this for hcds with big-endian registers
+ - big-endian-desc : boolean, set this for hcds with big-endian descriptors
+ - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
+ - clocks : a list of phandle + clock specifier pairs
+ - phys : phandle + phy specifier pair
+ - phy-names : "usb"
Example (Sequoia 440EPx):
ehci at e0000300 {
@@ -23,3 +24,13 @@ Example (Sequoia 440EPx):
reg = <0 e0000300 90 0 e0000390 70>;
big-endian;
};
+
+Example (Allwinner sun4i A10 SoC):
+ ehci0: ehci at 0x01c14000 {
+ compatible = "allwinner,sun4i-a10-ehci", "usb-ehci";
+ reg = <0x01c14000 0x100>;
+ interrupts = <39>;
+ clocks = <&ahb_gates 1>;
+ phys = <&usbphy 1>;
+ phy-names = "usb";
+ };
diff --git a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
deleted file mode 100644
index 17b3ad1..0000000
--- a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-VIA/Wondermedia VT8500 EHCI Controller
------------------------------------------------------
-
-Required properties:
-- compatible : "via,vt8500-ehci"
-- reg : Should contain 1 register ranges(address and length)
-- interrupts : ehci controller interrupt
-
-Example:
-
- ehci at d8007900 {
- compatible = "via,vt8500-ehci";
- reg = <0xd8007900 0x200>;
- interrupts = <43>;
- };
diff --git a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/vt8500-ehci.txt
deleted file mode 100644
index 5fb8fd6..0000000
--- a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-VIA VT8500 and Wondermedia WM8xxx SoC USB controllers.
-
-Required properties:
- - compatible: Should be "via,vt8500-ehci" or "wm,prizm-ehci".
- - reg: Address range of the ehci registers. size should be 0x200
- - interrupts: Should contain the ehci interrupt.
-
-usb: ehci at D8007100 {
- compatible = "wm,prizm-ehci", "usb-ehci";
- reg = <0xD8007100 0x200>;
- interrupts = <1>;
-};
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index a9707da..e28cbe0 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -255,6 +255,7 @@ config USB_EHCI_ATH79
config USB_EHCI_HCD_PLATFORM
tristate "Generic EHCI driver for a platform device"
+ depends on !PPC_OF
default n
---help---
Adds an EHCI host driver for a generic platform device, which
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 7f30b71..5c9ab74 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -3,6 +3,7 @@
*
* Copyright 2007 Steven Brown <sbrown@cortland.com>
* Copyright 2010-2012 Hauke Mehrtens <hauke@hauke-m.de>
+ * Copyright 2014 Hans de Goede <hdegoede@redhat.com>
*
* Derived from the ohci-ssb driver
* Copyright 2007 Michael Buesch <m@bues.ch>
@@ -18,6 +19,7 @@
*
* Licensed under the GNU/GPL. See COPYING for details.
*/
+#include <linux/clk.h>
#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/kernel.h>
@@ -25,6 +27,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>
@@ -33,6 +36,13 @@
#include "ehci.h"
#define DRIVER_DESC "EHCI generic platform driver"
+#define EHCI_MAX_CLKS 3
+#define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv)
+
+struct ehci_platform_priv {
+ struct clk *clks[EHCI_MAX_CLKS];
+ struct phy *phy;
+};
static const char hcd_name[] = "ehci-platform";
@@ -64,38 +74,90 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
return 0;
}
+static int ehci_platform_power_on(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+ int clk, ret;
+
+ for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) {
+ ret = clk_prepare_enable(priv->clks[clk]);
+ if (ret)
+ goto err_disable_clks;
+ }
+
+ if (priv->phy) {
+ ret = phy_init(priv->phy);
+ if (ret)
+ goto err_disable_clks;
+
+ ret = phy_power_on(priv->phy);
+ if (ret)
+ goto err_exit_phy;
+ }
+
+ return 0;
+
+err_exit_phy:
+ phy_exit(priv->phy);
+err_disable_clks:
+ while (--clk >= 0)
+ clk_disable_unprepare(priv->clks[clk]);
+
+ return ret;
+}
+
+static void ehci_platform_power_off(struct platform_device *dev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+ int clk;
+
+ if (priv->phy) {
+ phy_power_off(priv->phy);
+ phy_exit(priv->phy);
+ }
+
+ for (clk = EHCI_MAX_CLKS - 1; clk >= 0; clk--)
+ if (priv->clks[clk])
+ clk_disable_unprepare(priv->clks[clk]);
+}
+
static struct hc_driver __read_mostly ehci_platform_hc_driver;
static const struct ehci_driver_overrides platform_overrides __initconst = {
- .reset = ehci_platform_reset,
+ .reset = ehci_platform_reset,
+ .extra_priv_size = sizeof(struct ehci_platform_priv),
};
-static struct usb_ehci_pdata ehci_platform_defaults;
+static struct usb_ehci_pdata ehci_platform_defaults = {
+ .power_on = ehci_platform_power_on,
+ .power_suspend = ehci_platform_power_off,
+ .power_off = ehci_platform_power_off,
+};
static int ehci_platform_probe(struct platform_device *dev)
{
struct usb_hcd *hcd;
struct resource *res_mem;
- struct usb_ehci_pdata *pdata;
- int irq;
- int err;
+ struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
+ struct ehci_platform_priv *priv;
+ int err, irq, clk = 0;
if (usb_disabled())
return -ENODEV;
/*
- * use reasonable defaults so platforms don't have to provide these.
- * with DT probing on ARM, none of these are set.
+ * Use reasonable defaults so platforms don't have to provide these
+ * with DT probing on ARM.
*/
- if (!dev_get_platdata(&dev->dev))
- dev->dev.platform_data = &ehci_platform_defaults;
+ if (!pdata)
+ pdata = &ehci_platform_defaults;
err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
if (err)
return err;
- pdata = dev_get_platdata(&dev->dev);
-
irq = platform_get_irq(dev, 0);
if (irq < 0) {
dev_err(&dev->dev, "no irq provided");
@@ -107,17 +169,40 @@ static int ehci_platform_probe(struct platform_device *dev)
return -ENXIO;
}
+ hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
+ dev_name(&dev->dev));
+ if (!hcd)
+ return -ENOMEM;
+
+ platform_set_drvdata(dev, hcd);
+ dev->dev.platform_data = pdata;
+ priv = hcd_to_ehci_priv(hcd);
+
+ if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
+ priv->phy = devm_phy_get(&dev->dev, "usb");
+ if (IS_ERR(priv->phy)) {
+ err = PTR_ERR(priv->phy);
+ if (err == -EPROBE_DEFER)
+ goto err_put_hcd;
+ priv->phy = NULL;
+ }
+
+ for (clk = 0; clk < EHCI_MAX_CLKS; clk++) {
+ priv->clks[clk] = of_clk_get(dev->dev.of_node, clk);
+ if (IS_ERR(priv->clks[clk])) {
+ err = PTR_ERR(priv->clks[clk]);
+ if (err == -EPROBE_DEFER)
+ goto err_put_clks;
+ priv->clks[clk] = NULL;
+ break;
+ }
+ }
+ }
+
if (pdata->power_on) {
err = pdata->power_on(dev);
if (err < 0)
- return err;
- }
-
- hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
- dev_name(&dev->dev));
- if (!hcd) {
- err = -ENOMEM;
- goto err_power;
+ goto err_put_clks;
}
hcd->rsrc_start = res_mem->start;
@@ -126,21 +211,25 @@ static int ehci_platform_probe(struct platform_device *dev)
hcd->regs = devm_ioremap_resource(&dev->dev, res_mem);
if (IS_ERR(hcd->regs)) {
err = PTR_ERR(hcd->regs);
- goto err_put_hcd;
+ goto err_power;
}
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err)
- goto err_put_hcd;
-
- platform_set_drvdata(dev, hcd);
+ goto err_power;
return err;
-err_put_hcd:
- usb_put_hcd(hcd);
err_power:
if (pdata->power_off)
pdata->power_off(dev);
+err_put_clks:
+ while (--clk >= 0)
+ clk_put(priv->clks[clk]);
+err_put_hcd:
+ if (pdata == &ehci_platform_defaults)
+ dev->dev.platform_data = NULL;
+
+ usb_put_hcd(hcd);
return err;
}
@@ -149,13 +238,19 @@ static int ehci_platform_remove(struct platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
+ struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+ int clk;
usb_remove_hcd(hcd);
- usb_put_hcd(hcd);
if (pdata->power_off)
pdata->power_off(dev);
+ for (clk = 0; priv->clks[clk] && clk < EHCI_MAX_CLKS; clk++)
+ clk_put(priv->clks[clk]);
+
+ usb_put_hcd(hcd);
+
if (pdata == &ehci_platform_defaults)
dev->dev.platform_data = NULL;
@@ -206,8 +301,10 @@ static int ehci_platform_resume(struct device *dev)
static const struct of_device_id vt8500_ehci_ids[] = {
{ .compatible = "via,vt8500-ehci", },
{ .compatible = "wm,prizm-ehci", },
+ { .compatible = "usb-ehci", },
{}
};
+MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
static const struct platform_device_id ehci_platform_table[] = {
{ "ehci-platform", 0 },
--
1.8.4.2
^ permalink raw reply related
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
From: Mark Rutland @ 2014-01-15 16:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140115134653.GF25824@e106331-lin.cambridge.arm.com>
>
> Another, more invasive option would be extend the dts syntax and teach
> dtc to handle property appending. Then the soc dts could stay as it is,
> and the board dts could have something like:
>
> /append-property/ interrupts = <&intc1 6 1>;
> /append-property/ interrupt-names = "board-specific-irq";
>
> Both these options solve the issue at the source, are general to other
> properties, and allow more than one level of hierarchy (the proposed
> interrupts-extended-2 only allows one level).
I've just had a go at implementing the append-property mechanism above
in dtc, and it was far easier than I expected (patch below).
Does anyone have any issues with the /append-property/ idea?
Thanks,
Mark.
---->8----
>From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Wed, 15 Jan 2014 15:43:51 +0000
Subject: [PATCH] dtc: add ability to append properties
When dealing with hierarchies of dtsi files, handling minute differences
between individual boards can become very painful. Adding a single
board-specific interrupt requires duplicating the entire interrupts
property, which requires duplication of common values. This makes bug
fixing painful and if not handled very carefully files diverge rapdily.
To ameliorate this, this patch adds the ability to append properties,
allowing board files to describe only the additional values required in
a property. This functionality also works with strings, so parallel
properties like interupts and interrupt-names stay in sync. Properties
may be appended multiple times, and deleting properties clears all
previous appended values.
To append a property, secondary definitions must be prefixed with
/append-property/. This is longer than a possible '+=' syntax, but makes
it far easier to spot when appending behaviour is requested, and so
hopefully will lead to fewer buggy dts files.
For example, if the following dts fragements are compiled together:
/ {
interrupts = <0>, <1>;
interrupt-names = "zero", "one";
};
/ {
/append-property/ interrupts = <2>, <3>;
/append-property/ interrupt-names = "two", "three";
};
They will result in a dtb equivalent to the following dts fragment:
/ {
interrupts = <0>, <1>, <2>, <3>;
interrupt-names = "zero", "one", "two", "three";
};
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
dtc-lexer.l | 7 +++++++
dtc-parser.y | 5 +++++
dtc.h | 2 ++
livetree.c | 15 ++++++++++++++-
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 0cd7e67..7989abe 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -131,6 +131,13 @@ static bool pop_input_file(void);
return DT_DEL_PROP;
}
+<*>"/append-property/" {
+ DPRINT("Keyword: /append-property/\n");
+ DPRINT("<PROPNODENAME>\n");
+ BEGIN(PROPNODENAME);
+ return DT_APPEND_PROP;
+ }
+
<*>"/delete-node/" {
DPRINT("Keyword: /delete-node/\n");
DPRINT("<PROPNODENAME>\n");
diff --git a/dtc-parser.y b/dtc-parser.y
index 4864631..a8409ed 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -63,6 +63,7 @@ static unsigned char eval_char_literal(const char *s);
%token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
%token DT_BITS
%token DT_DEL_PROP
+%token DT_APPEND_PROP
%token DT_DEL_NODE
%token <propnodename> DT_PROPNODENAME
%token <literal> DT_LITERAL
@@ -195,6 +196,10 @@ propdef:
{
$$ = build_property($1, empty_data);
}
+ | DT_APPEND_PROP DT_PROPNODENAME '=' propdata ';'
+ {
+ $$ = build_property_append($2, $4);
+ }
| DT_DEL_PROP DT_PROPNODENAME ';'
{
$$ = build_property_delete($2);
diff --git a/dtc.h b/dtc.h
index 20e4d56..8687530 100644
--- a/dtc.h
+++ b/dtc.h
@@ -133,6 +133,7 @@ struct label {
};
struct property {
+ bool appended;
bool deleted;
char *name;
struct data val;
@@ -186,6 +187,7 @@ void delete_labels(struct label **labels);
struct property *build_property(char *name, struct data val);
struct property *build_property_delete(char *name);
+struct property *build_property_append(char *name, struct data val);
struct property *chain_property(struct property *first, struct property *list);
struct property *reverse_properties(struct property *first);
diff --git a/livetree.c b/livetree.c
index b61465f..894e42b 100644
--- a/livetree.c
+++ b/livetree.c
@@ -74,6 +74,15 @@ struct property *build_property_delete(char *name)
return new;
}
+struct property *build_property_append(char *name, struct data val)
+{
+ struct property *new = build_property(name, val);
+
+ new->appended = 1;
+
+ return new;
+}
+
struct property *chain_property(struct property *first, struct property *list)
{
assert(first->next == NULL);
@@ -167,7 +176,11 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
for_each_label_withdel(new_prop->labels, l)
add_label(&old_prop->labels, l->label);
- old_prop->val = new_prop->val;
+ if (new_prop->appended)
+ old_prop->val = data_merge(old_prop->val, new_prop->val);
+ else
+ old_prop->val = new_prop->val;
+
old_prop->deleted = 0;
free(new_prop);
new_prop = NULL;
--
1.8.1.1
^ permalink raw reply related
* [PATCH 1/2] clk: hisilicon: add hi3620_mmc_clks
From: Kevin Hilman @ 2014-01-15 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140115082908.4167.34040@quantum>
Mike Turquette <mturquette@linaro.org> writes:
> Quoting Haojian Zhuang (2014-01-14 21:59:40)
>>
>> On 01/15/2014 11:53 AM, Mike Turquette wrote:
>> > Quoting zhangfei (2014-01-14 17:40:25)
>> >> Dear Mike
>> >>
>> >> On 01/15/2014 04:17 AM, Mike Turquette wrote:
>> >>> Quoting Zhangfei Gao (2014-01-13 01:14:28)
>> >>>> Suggest by Arnd: abstract mmc tuning as clock behavior,
>> >>>> also because different soc have different tuning method and registers.
>> >>>> hi3620_mmc_clks is added to handle mmc clock specifically on hi3620.
>> >>>>
>> >>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> >>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> >>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>> >>> Patch looks good to me with one exception. I do not have
>> >>> Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt in the
>> >>> clk-next branch. Is there a stable branch I can pull in as a dependency?
>> >> Mach-hisi just have been uploaeded.
>> >> Have tried next-20140114, the patch can be applied successfully.
>> >> While v3.13-rc8 still can not.
>> >>
>> >> Is this fine?
>> > Can you give me a link to the branch that introduces
>> > Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt?
>> >
>> > I guess the patch introducing it is going through arm-soc. Is this going
>> > in for 3.14? If so then perhaps the clk tree and the arm-soc tree can
>> > share a stable branch that introduces it.
>> >
>> > Regards,
>> > Mike
>> >
>> Some patches are merged into arm-soc, and others are in clk tree.
>> If sharing a stable branch between arm-soc and clk tree, it only means
>> that we need to revert all commits that are in arm-soc and clk tree.
>> I think it's too complex.
>
> I'm suggesting reverting any patches that are applied to arm-soc. I'm
> only suggesting that there might be a common branch that both the clk
> and arm-soc trees can depend on to fix this problem.
This one is stable, and is where the binding is introduced:
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git hisi/soc
>> How about split the patch? The patch on document should enter in arm-soc.
>
> That is one approach. You might want to run it past the arm-soc folks
> first to see if they will take in the binding definition for 3.14.
We're not taking anything new for 3.14.
Kevin
^ permalink raw reply
* [PATCH v1 2/5] pinctrl: st: Add Interrupt support.
From: srinivas kandagatla @ 2014-01-15 16:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdY=uTtE8CMyQocQcV4c7xmvR_it1ue03kSfKmaLQX_sxA@mail.gmail.com>
Thankyou for the detailed comments.
On 15/01/14 14:15, Linus Walleij wrote:
> On Tue, Jan 14, 2014 at 3:52 PM, <srinivas.kandagatla@st.com> wrote:
>
>> ST Pincontroller GPIO bank can have one of the two possible types of
>> interrupt-wirings.
>
> Interesting :-)
>
>> +Pin controller node:
>> +Required properties:
>> - compatible : should be "st,<SOC>-<pio-block>-pinctrl"
>> like st,stih415-sbc-pinctrl, st,stih415-front-pinctrl and so on.
>> -- gpio-controller : Indicates this device is a GPIO controller
>> -- #gpio-cells : Should be one. The first cell is the pin number.
>> +- st,syscfg : Should be a phandle of the syscfg node.
>
> So why do you add this? This is totally unused by your driver.
The document got re-ordered by adding new information.
st,syscfg is not a new field, we use it for getting hold of regmap instance.
I will re-check if this addition was due to some white spaces.
>
>> - st,retime-pin-mask : Should be mask to specify which pins can be retimed.
>> If the property is not present, it is assumed that all the pins in the
>> bank are capable of retiming. Retiming is mainly used to improve the
>> IO timing margins of external synchronous interfaces.
>> -- st,bank-name : Should be a name string for this bank as
>> - specified in datasheet.
>
> Why do you have this property? The driver is not using it.
Its used for bank label.
>
> And what is wrong with just using that name for the node?
On ST chips the gpio banks are named irregularly in some instances.
They are named like PIO0, PIO1 , PIO100, PIO101 and so on. Same naming
is used across board schematics too, so Its easy for debugging if we
have this consistency in the driver as well.
As node names are in pio at address format, which are not same as
bank-names in the datasheet.
>
>> -- st,syscfg : Should be a phandle of the syscfg node.
>> +- ranges : specifies the ranges for the pin controller memory.
>
> And what are they used for? I've never seen this before.
This was suggested when I first submitted the driver.
The intention here was to describe the mapping between pinctrl (parent)
node and the gpio bank(children) nodes.
I will fix the document for this too in next version.
>
>> +Optional properties:
>> +- interrupts : Interrupt number of the irqmux. If the interrupt is shared
>> + with other gpio banks via irqmux.
>> + a irqline and gpio banks.
>> +- reg : irqmux memory resource. If irqmux is present.
>> +- reg-names : irqmux resource should be named as "irqmux".
>> +
>> +GPIO controller node.
>> +Required properties:
>> +- gpio-controller : Indicates this device is a GPIO controller
>> +- #gpio-cells : Should be one. The first cell is the pin number.
>> +- st,bank-name : Should be a name string for this bank as specified in
>> + datasheet.
>
> Again, why?
>
>> +Optional properties:
>> +- interrupts : Interrupt number for this gpio bank. If there is a dedicated
>> + interrupt wired up for this gpio bank.
>> +
>> +- interrupt-controller : Indicates this device is a interrupt controller. GPIO
>> + bank can be an interrupt controller iff one of the interrupt type either via
>> +irqmux or a dedicated interrupt per bank is specified.
>> +
>> +- #interrupt-cells: the value of this property should be 2.
>> + - First Cell: represents the external gpio interrupt number local to the
>> + external gpio interrupt space of the controller.
>> + - Second Cell: flags to identify the type of the interrupt
>> + - 1 = rising edge triggered
>> + - 2 = falling edge triggered
>> + - 3 = rising and falling edge triggered
>> + - 4 = high level triggered
>> + - 8 = low level triggered
>
> Correct, but reference symbols from:
> include/dt-bindings/interrupt-controller/irq.h
> in example.
Ok.
>
>
>>
>> Example:
>> pin-controller-sbc {
>
> Please put in an updated example making use of all the
> new props.
Will do it in next version.
>
>
> (...)
>> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
>
>> @@ -271,6 +276,8 @@ struct st_gpio_bank {
>> struct pinctrl_gpio_range range;
>> void __iomem *base;
>> struct st_pio_control pc;
>> + struct irq_domain *domain;
>> + int gpio_irq;
>
> Why are you putting this IRQ into the state container when it can be
> a function local variable in probe()?
I agree, this should be a local variable.
>
>> struct st_pinctrl {
>> @@ -284,6 +291,8 @@ struct st_pinctrl {
>> int ngroups;
>> struct regmap *regmap;
>> const struct st_pctl_data *data;
>> + void __iomem *irqmux_base;
>> + int irqmux_irq;
>
> Dito. I think.
I agree, this should be a local variable too.
>
>> +static int st_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>> +{
>> + struct st_gpio_bank *bank = gpio_chip_to_bank(chip);
>> + int virq;
>
> Just name this variable "irq". It is no more virtual than any other
> IRQ.
Ok.
>
>> +
>> + if (bank->domain && (offset < chip->ngpio))
>> + virq = irq_create_mapping(bank->domain, offset);
>
> No, don't do the create_mapping() call in the .to_irq() function.
> Call irq_create_mapping() for *all* valid hwirqs in probe() and use
> irq_find_mapping() here.
Ok.
>
>> +static void st_gpio_irq_disable(struct irq_data *d)
>> +{
>> + struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
>> +
>> + writel(BIT(d->hwirq), bank->base + REG_PIO_CLR_PMASK);
>> +}
>> +
>> +static void st_gpio_irq_enable(struct irq_data *d)
>> +{
>> + struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
>> +
>> + writel(BIT(d->hwirq), bank->base + REG_PIO_SET_PMASK);
>> +}
>
> I *strongly* suspect that these two should be replaced with
> _mask()/_unmask() rather than using disable/enable.
>
> Because that seems to be what they are doing.
I will do this change in next version.
>
> (...)
>> +static void __gpio_irq_handler(struct st_gpio_bank *bank)
>> +{
>> + unsigned long port_in, port_mask, port_comp, port_active;
>> + int n;
>> +
>> + port_in = readl(bank->base + REG_PIO_PIN);
>> + port_comp = readl(bank->base + REG_PIO_PCOMP);
>> + port_mask = readl(bank->base + REG_PIO_PMASK);
>> +
>> + port_active = (port_in ^ port_comp) & port_mask;
>> +
>> + for_each_set_bit(n, &port_active, BITS_PER_LONG) {
>> + generic_handle_irq(irq_find_mapping(bank->domain, n));
>
> So what happens if new IRQs appear in the register while you are
> inside this loop?
I agree, there is a possibility of missing interrupt here, doing similar
to what you suggested makes sense.
>
> Check this recent patch:
> http://marc.info/?l=linux-arm-kernel&m=138979164119464&w=2
>
> Especially this:
>
> + for (;;) {
> + mask = ioread8(host->base + CLRHILVINT) & 0xff;
> + mask |= (ioread8(host->base + SECOINT) & SECOINT_MASK) << 8;
> + mask |= (ioread8(host->base + PRIMINT) & PRIMINT_MASK) << 8;
> + mask &= host->irq_high_enabled | (host->irq_sys_enabled << 8);
> + if (mask == 0)
> + break;
> + for_each_set_bit(n, &mask, BITS_PER_LONG)
> + generic_handle_irq(irq_find_mapping(host->domain, n));
> + }
>
>
>> +static struct irq_chip st_gpio_irqchip = {
>> + .name = "GPIO",
>> + .irq_disable = st_gpio_irq_disable,
>> + .irq_mask = st_gpio_irq_disable,
>> + .irq_unmask = st_gpio_irq_enable,
>
> Just implement mask/unmask as indicated earlier.
>
>> + .irq_set_type = st_gpio_irq_set_type,
>> +};
>
> You need to mark IRQ GPIO lines as used for IRQ.
>
> Add startup() and shutdown() hooks similar to those I add
> in this patch:
>
> http://marc.info/?l=linux-gpio&m=138977709114785&w=2
Will add this in next version.
>
>> +static int st_gpio_irq_domain_xlate(struct irq_domain *d,
>> + struct device_node *ctrlr, const u32 *intspec, unsigned int intsize,
>> + irq_hw_number_t *out_hwirq, unsigned int *out_type)
>> +{
>> + struct st_gpio_bank *bank = d->host_data;
>> + int ret;
>> + int pin = bank->gpio_chip.base + intspec[0];
>> +
>> + if (WARN_ON(intsize < 2))
>> + return -EINVAL;
>> +
>> + *out_hwirq = intspec[0];
>> + *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
>> +
>> + ret = gpio_request(pin, ctrlr->full_name);
>> + if (ret)
>> + return ret;
>> +
>> + ret = gpio_direction_input(pin);
>> + if (ret)
>> + return ret;
>
> We recently concluded that you should *NOT* call
> gpio_request() and gpio_direction_input() from xlate or similar.
Great, I can get rid of this function and use generic xlate function
instead.
>
> Instead: set up the hardware directly in mask/unmask callbacks
> so that the irqchip can trigger IRQs directly without any
> interaction with the GPIO subsystem.
Ok, got it.
>
> By implementing the startup/shutdown hooks as indicated
> above you can still indicate to the GPIO subsystem what is
> going on and it will enforce that e.g. the line is not set to
> output.
>
Ok.
>> + bank->domain = irq_domain_add_linear(np,
>> + ST_GPIO_PINS_PER_BANK,
>> + &st_gpio_irq_ops, bank);
>> + if (!bank->domain)
>> + dev_err(dev, "Failed to add irq domain for [%s]\n",
>> + np->full_name);
>> + } else {
>> + dev_info(dev, "No IRQ support for [%s] bank\n", np->full_name);
>
> Why is the [bank name] inside [brackets]?
There is no strong reason, I will remove it.
>
> Yours,
> Linus Walleij
>
>
^ permalink raw reply
* [PATCH] mmc: dw_mmc: fix dw_mci_get_cd
From: Russell King - ARM Linux @ 2014-01-15 16:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <15344443.B6eHZOpkXH@wuerfel>
On Wed, Jan 15, 2014 at 05:07:49PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 January 2014 16:01:46 Russell King - ARM Linux wrote:
> > On Wed, Jan 15, 2014 at 02:59:59PM +0100, Arnd Bergmann wrote:
> > > On Wednesday 15 January 2014 21:56:26 zhangfei wrote:
> > > > However, in our board cd =0 when card is deteced while cd=1 when card is
> > > > removed.
> > > > In order to mmc_gpio_get_cd return 1, MMC_CAP2_CD_ACTIVE_HIGH has to be
> > > > set, as well as new property "caps2-mmc-cd-active-low".
> > > >
> > > > --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> > > > +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> > > > @@ -73,6 +73,8 @@ Optional properties:
> > > > +* caps2-mmc-cd-active-low: cd pin is low when card active
> > > > +
> > > >
> > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > > > + if (of_find_property(np, "caps2-mmc-cd-active-low", NULL))
> > > > + pdata->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> > > > +
> > >
> > > The MMC_CAP2_CD_ACTIVE_HIGH flag should only be required for
> > > legacy platforms. With DT parsing, you can normally specify
> > > the polarity of the GPIO line in the GPIO specifier in DT.
> >
> > Since when?
>
> I just looked through the bindings and found that about half
> of them allow passing polarity in the gpio specifier. I thought
> it was more than that and IIRC the outcome of a previous discussion
> was that when the gpio controller allows passing polarity, you
> should use that rather than a separate flag.
>
> Not a problem though, since we can use the cd-inverted and
> wp-inverted properties.
Well, having this in the gpio level as well as in subsystems like MMC
is going to create confusion - and from the results of this patch _IS_
causing confusion. You've just encouraged one implementation to have
things setup such that mmc_gpio_get_cd() returns false when a card is
inserted, rather than it correctly returning true.
The issue here is that we'll potentially now end up with _three_
inversions. One in the GPIO layers. One in mmc_gpio_get_cd(), and
another in every driver which uses the GPIO layer inversion. This
is hardly the right approach as it leads to multiple points where
confusion about the inverted status can occur.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH v4 05/16] ARM: use a function table for determining instruction interpreter actions
From: David Long @ 2014-01-15 16:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1387543554.3404.34.camel@linaro1.home>
On 12/20/13 07:45, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Make the instruction interpreter call back to semantic action functions
>> through a function pointer array provided by the invoker. The interpreter
>> decodes the instructions into groups and uses the group number to index
>> into the supplied array. kprobes and uprobes code will each supply their
>> own array of functions.
>>
>> Signed-off-by: David A. Long <dave.long@linaro.org>
>> ---
>
> Because I've been very slow in reviewing these I've only just noticed
> that some of the the comments I made on version one of this patch didn't
> get a response. I've copied them again below (slightly edited) and
> heavily trimmed the patch...
>
>> arch/arm/kernel/kprobes-arm.c | 41 +++++++++++
>> arch/arm/kernel/kprobes-common.c | 3 +-
>> arch/arm/kernel/kprobes-thumb.c | 92 ++++++++++++++++++------
>> arch/arm/kernel/kprobes.c | 10 ++-
>> arch/arm/kernel/kprobes.h | 14 ++--
>> arch/arm/kernel/probes-arm.c | 114 +++++++++++++++---------------
>> arch/arm/kernel/probes-arm.h | 37 ++++++++++
>> arch/arm/kernel/probes-thumb.c | 149 +++++++++++++++++++--------------------
>> arch/arm/kernel/probes-thumb.h | 14 ++--
>> arch/arm/kernel/probes.c | 13 ++--
>> arch/arm/kernel/probes.h | 15 ++--
>> 11 files changed, 325 insertions(+), 177 deletions(-)
>>
>> diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
>> index a359475..ee329ff 100644
>> --- a/arch/arm/kernel/kprobes-arm.c
>> +++ b/arch/arm/kernel/kprobes-arm.c
>> @@ -299,3 +299,44 @@ emulate_rdlo12rdhi16rn0rm8_rwflags_nopc(struct kprobe *p, struct pt_regs *regs)
>> regs->uregs[rdhi] = rdhiv;
>> regs->ARM_cpsr = (regs->ARM_cpsr & ~APSR_MASK) | (cpsr & APSR_MASK);
>> }
>> +
>> +const union decode_item kprobes_arm_actions[] = {
>
> I think it's best if we don't reuse the decode_item type here, this is a
> different sort of table so probably best to have it's own union. Also,
> if we do that, then decode_item can be simplified as it won't need to
> have function pointers in it, i.e. we could end up with...
>
> union decode_action {
> kprobe_insn_handler_t *handler;
> kprobe_custom_decode_t *decoder;
> };
>
> union decode_item {
> u32 bits;
> const union decode_item *table;
> };
>
> typedef enum kprobe_insn (kprobe_custom_decode_t)(kprobe_opcode_t,
> struct arch_specific_insn *,
> union decode_action *actions);
>
>
I've added the following:
typedef enum kprobe_insn (probes_custom_decode_t)(kprobe_opcode_t,
struct arch_specific_insn *,
struct decode_header *);
union decode_action {
kprobe_insn_handler_t *handler;
probes_custom_decode_t *decoder;
};
Note the third argument actually passed into the decoder functions is
the decode table entry. decode_action is only used to select a
decode/emulate/simullate function.
> A second point, I think it would be a good idea to make sure these
> action arrays are the size we expect by adding an entry at the end of
> the relevant enumeration and using that to set the size of the arrays.
> E.g. for this one
>
> enum probes_arm_action {
> ...
> ...
> NUM_PROBES_ARM_ACTIONS
> };
>
> and then use it like:
>
> const union decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
>
> That way, we at least make any uninitialised entries are null (I
> assume?) which is safer than accidentally indexing beyond the array.
>
>
Done.
>> + [PROBES_EMULATE_NONE] = {.handler = kprobe_emulate_none},
>> + [PROBES_SIMULATE_NOP] = {.handler = kprobe_simulate_nop},
>> + [PROBES_PRELOAD_IMM] = {.handler = kprobe_simulate_nop},
>
> [...]
>
>
>> diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
>> index d14d224..2238972 100644
>> --- a/arch/arm/kernel/probes.h
>> +++ b/arch/arm/kernel/probes.h
>> @@ -131,7 +131,8 @@ void __kprobes kprobe_simulate_nop(struct kprobe *p, struct pt_regs *regs);
>> void __kprobes kprobe_emulate_none(struct kprobe *p, struct pt_regs *regs);
>>
>> enum kprobe_insn __kprobes
>> -kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi);
>> +kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_specific_insn *asi,
>> + struct decode_header *h);
>>
>> /*
>> * Test if load/store instructions writeback the address register.
>> @@ -334,7 +335,7 @@ struct decode_custom {
>>
>> #define DECODE_CUSTOM(_mask, _value, _decoder) \
>> DECODE_HEADER(DECODE_TYPE_CUSTOM, _mask, _value, 0), \
>> - {.decoder = (_decoder)}
>> + {.bits = (_decoder)}
>>
>
> This third and final comment is probably just bike shedding...
>
> 'bits' looks a bit funny here. I've been trying to think of a way of
> making it nicer but it's difficult. The actual value is one of three
> different enums, so if we were to add another members to decode_item it
> would just have to be "int action", at least that would read nicer in
> these macros and where it gets read out in probes_decode_insn.
>
I agree. I've added an "int action" to the decode_item union, and use
it instead of "bits" for the action array index.
I've also updated the description of how this all works, in the comments.
-dl
^ permalink raw reply
* [alsa-devel] [PATCH RFC v2 REPOST 3/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
From: Jyri Sarha @ 2014-01-15 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52D691AD.3010206@iki.fi>
On 01/15/2014 03:48 PM, Anssi Hannula wrote:
> 15.01.2014 13:27, Jyri Sarha kirjoitti:
>> On 12/31/2013 03:25 PM, Mark Brown wrote:
>>> On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote:
>>>> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE.
>>>> The 8 least significant bits are ignored.
>>>
>>> Where does this constraint come from?
>>>
>>
>> From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N
>> register statically to a value that works only with 4 byte samples.
>> According to my tests it is possible to support 3 and 2 byte samples too
>> by changing the CTS_N register value, but I am not sure if the
>> configuration can be changed on the fly. My data sheet of the nxp chip
>> is very vague about the register definitions, but I suppose the register
>> configures some clock divider on the chip. HDMI supports only upto 24bit
>> audio and the data sheet states that any extraneous least significant
>> bits are ignored.
>
> That sounds strange, CTS/N values only depend on audio sample rate and
> TMDS/video clock, not on the audio format or the size of samples (HDMI
> spec sec 7.2 - Audio Sample Clock Capture and Regeneration).
>
> Sure there isn't anything more going on (like the used HDMI sink being
> more tolerant to wrong CTS/N with 4-byte samples, or more likely
> something else I can't immediately think of)?
>
On theoretical level I am not really sure about anything, because have
not been able to get my hands on proper NXP TDA19988 documentation.
However, while playing around with my Beaglebone-Black I have have found
out in practice that by changing CTS_N register's (page 0x11 reg 0x0c)
K_SEL bits (bit 2 to 0)[1] I can get 2, 3 and 4 byte samples to work
consistently by setting:
K_SEL = 1 for SNDRV_PCM_FORMAT_S16_LE
K_SEL = 2 for SNDRV_PCM_FORMAT_S24_3LE
K_SEL = 3 for SNDRV_PCM_FORMAT_S32_LE
Because I do not really know what is going on, I did not want to suggest
any changes to the driver and just use the format that works with the
current driver version.
The HDMI sinks I have been using are a Toshiba 22B2LF1G and Thomson
42E90NF32 televisions. About those I just know that they both behave the
same way.
Best regards,
Jyri
[1] The closest data sheet that has any description about the chip
registers I have found is this:
http://media.digikey.com/pdf/Data%20Sheets/NXP%20PDFs/TDA9983B.pdf
^ permalink raw reply
* [PATCH v6 1/2] ohci-platform: Add support for devicetree instantiation
From: Alan Stern @ 2014-01-15 16:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389802104-17330-2-git-send-email-hdegoede@redhat.com>
On Wed, 15 Jan 2014, Hans de Goede wrote:
> +static int ohci_platform_power_on(struct platform_device *dev)
> +{
> + struct usb_hcd *hcd = platform_get_drvdata(dev);
> + struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> + int clk, ret;
> +
> + for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) {
You fixed this one...
> @@ -125,13 +225,22 @@ static int ohci_platform_remove(struct platform_device *dev)
> {
> struct usb_hcd *hcd = platform_get_drvdata(dev);
> struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
> + struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> + int clk;
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(hcd);
>
> if (pdata->power_off)
> pdata->power_off(dev);
>
> + for (clk = 0; priv->clks[clk] && clk < OHCI_MAX_CLKS; clk++)
but not this one. :-( Same for the ehci-platform patch.
In fact, it might be easier to make this loop go backward, like
you do in the *_platform_power_off routines.
Is there a devm-type routine that will take care of all this for you?
Alan Stern
^ permalink raw reply
* [PATCHv12] ARM: dts: Add support for the cpuimx35 board from Eukrea and its baseboard.
From: Denis Carikli @ 2014-01-15 16:32 UTC (permalink / raw)
To: linux-arm-kernel
The following devices/functionalities were added:
* Main and secondary UARTs.
* i2c and the pcf8563 device.
* Ethernet.
* NAND.
* The BP1 button.
* The LED.
* Watchdog
* SD.
Cc: Eric B?nard <eric@eukrea.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
ChangeLog v11->v12:
- Moved the pinctrl properties of the led1 and bp1 nodes to their respective parent nodes.
ChangeLog v10->v11:
- The other patches in this serie have already been applied in the for-next branch
in Shawn Guo's tree at git://git.linaro.org/people/shawnguo/linux-2.6.git :
4449d01 ARM i.MX35: build in pinctrl support.
cd01fd4 ARM: imx_v6_v7_defconfig: Enable some drivers used on the cpuimx35.
- Reordered the Cc list and dropped the Cc to the devicetree binding maintainers.
- Moved the pins of the hoggrp inside their respective node, the bp1 and led1 nodes
were updated accordingly.
- The esdhc1's gpio was fixed: the card detect code will never uses the gpio "active"
high/low setting, instead it uses that GPIO as an IRQ.
- Cosmetic fixes in the uart pinctrl nodes.
ChangeLog v9->v10:
- Added Fabio Estevam To the Cc list
ChangeLog v8->v9:
- Added the dtb target into arch/arm/boot/dts/Makefile.
- Added a blank line before the pcf8563 node in the i2c1 node.
- Whitespace cleanups as requested by Shawn Guo.
- _' was used in label, it's now fixed (it has been replaced with '-').
- The recently introduced defines for input and gpios are now used.
- The uarts had the fsl,uart-has-rtscts property set, but they didn't use
any rts and cts pins in the uart pinctrl nodes.
ChangeLog v7->v8:
- The commit message was improved.
- The board specific iomuxc pins group configuration that were in imx35.dtsi were moved here.
- The rest of the patch was updated accordingly.
ChangeLog v6->v7:
- Added Grant Likely in the Cc list.
- Shortened the license.
- The tsc2007 pads were moved in another patch.
ChangeLog v5->v6:
- Shawn Guo was added in the Cc.
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi | 59 +++++++++
.../boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts | 137 ++++++++++++++++++++
3 files changed, 197 insertions(+)
create mode 100644 arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 570dcd6..98d3921 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -145,6 +145,7 @@ dtb-$(CONFIG_ARCH_MXC) += \
imx27-phytec-phycard-s-som.dtb \
imx27-phytec-phycard-s-rdk.dtb \
imx31-bug.dtb \
+ imx35-eukrea-mbimxsd35-baseboard.dtb \
imx50-evk.dtb \
imx51-apf51.dtb \
imx51-apf51dev.dtb \
diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
new file mode 100644
index 0000000..303f789
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * 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 "imx35.dtsi"
+
+/ {
+ model = "Eukrea CPUIMX35";
+ compatible = "eukrea,cpuimx35", "fsl,imx35";
+
+ memory {
+ reg = <0x80000000 0x8000000>; /* 128M */
+ };
+};
+
+&fec {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_fec>;
+ status = "okay";
+};
+
+&i2c1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c1>;
+ status = "okay";
+
+ pcf8563 at 51 {
+ compatible = "nxp,pcf8563";
+ reg = <0x51>;
+ };
+};
+
+&iomuxc {
+ imx35-eukrea {
+ pinctrl_fec: fecgrp {
+ fsl,pins = <MX35_FEC_PINGRP1>;
+ };
+
+ pinctrl_i2c1: i2c1grp {
+ fsl,pins = <MX35_I2C1_PINGRP1>;
+ };
+ };
+};
+
+&nfc {
+ nand-bus-width = <8>;
+ nand-ecc-mode = "hw";
+ nand-on-flash-bbt;
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts
new file mode 100644
index 0000000..23f6fe1
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts
@@ -0,0 +1,137 @@
+/*
+ * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * 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.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "imx35-eukrea-cpuimx35.dtsi"
+
+/ {
+ model = "Eukrea CPUIMX35";
+ compatible = "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+
+ gpio_keys {
+ compatible = "gpio-keys";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_bp1>;
+
+ bp1 {
+ label = "BP1";
+ gpios = <&gpio3 25 GPIO_ACTIVE_LOW>;
+ linux,code = <BTN_MISC>;
+ gpio-key,wakeup;
+ linux,input-type = <1>;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_led1>;
+
+ led1 {
+ label = "led1";
+ gpios = <&gpio3 29 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+};
+
+&audmux {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_audmux>;
+ status = "okay";
+};
+
+&esdhc1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_esdhc1>;
+ cd-gpios = <&gpio3 24>;
+ status = "okay";
+};
+
+&i2c1 {
+ tlv320aic23: codec at 1a {
+ compatible = "ti,tlv320aic23";
+ reg = <0x1a>;
+ };
+};
+
+&iomuxc {
+ imx35-eukrea {
+ pinctrl_audmux: audmuxgrp {
+ fsl,pins = <MX35_AUDMUX_PINGRP1>;
+ };
+
+ pinctrl_bp1: bp1grp {
+ fsl,pins = <MX35_PAD_LD19__GPIO3_25 0x80000000>;
+ };
+
+ pinctrl_esdhc1: esdhc1grp {
+ fsl,pins = <
+ MX35_ESDHC1_PINGRP1
+ MX35_PAD_LD18__GPIO3_24 0x80000000 /* CD */
+ >;
+ };
+
+ pinctrl_fec: fecgrp {
+ fsl,pins = <MX35_FEC_PINGRP1>;
+ };
+
+ pinctrl_i2c1: i2c1grp {
+ fsl,pins = <MX35_I2C1_PINGRP1>;
+ };
+
+ pinctrl_led1: led1grp {
+ fsl,pins = <MX35_PAD_LD23__GPIO3_29 0x80000000>;
+ };
+
+ pinctrl_reg_lcd_3v3: reg-lcd-3v3 {
+ fsl,pins = <MX35_PAD_D3_CLS__GPIO1_4 0x80000000>;
+ };
+
+ pinctrl_uart1: uart1grp {
+ fsl,pins = <
+ MX35_UART1_PINGRP1
+ MX35_UART1_RTSCTS_PINGRP1
+ >;
+ };
+
+ pinctrl_uart2: uart2grp {
+ fsl,pins = <
+ MX35_UART2_PINGRP1
+ MX35_UART2_RTSCTS_PINGRP1
+ >;
+ };
+ };
+};
+
+&ssi1 {
+ fsl,mode = "i2s-slave";
+ status = "okay";
+};
+
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart1>;
+ fsl,uart-has-rtscts;
+ status = "okay";
+};
+
+&uart2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart2>;
+ fsl,uart-has-rtscts;
+ status = "okay";
+};
--
1.7.9.5
^ permalink raw reply related
* [PATCH] ARM: Kirkwood: Add DT description of QNAP 419
From: Arnd Bergmann @ 2014-01-15 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140111210443.GT9681@lunn.ch>
On Saturday 11 January 2014, Andrew Lunn wrote:
> The id of -1 causes platform_device_add() to set the device name to
> plain "gpio-keys".
>
> When using DT, the device name is created by the function
> of_device_make_bus_id(). It has the following comment:
>
> * This routine will first try using either the dcr-reg or the reg property
> * value to derive a unique name. As a last resort it will use the node
> * name followed by a unique number.
>
> Since the gpio_keys node does not have a reg properties, it gets a
> unique number appended to it. We end up with the device name
> "gpio_keys.3"
>
> So as it stands, it does not appear i can make the DT system use the
> same device name as a board system.
>
> But i'm also a little bit concerned by the "unique number" and this
> ending up in /dev/input/by-path/platform-gpio_keys.3-event. Is this
> path supposed to be stable? This unique number is not stable. An
> unwitting change to the DT could cause its value to change. Do we need
> to make it stable?
>
One possibility would be to create an artificial bus in DT for all
gpio-keys devices, use #address-cells=<1> and #size-cells=<0>
for it, and give each device a unique reg property. Not sure if that
would be considered an elegant solution though.
Arnd
^ permalink raw reply
* [PATCH v4 5/6] spmi: document the PMIC arbiter SPMI bindings
From: Josh Cartwright @ 2014-01-15 16:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140115001344.GE23276@sonymobile.com>
On Tue, Jan 14, 2014 at 04:13:45PM -0800, Courtney Cavin wrote:
> On Tue, Jan 14, 2014 at 07:41:39PM +0100, Josh Cartwright wrote:
> > Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> > ---
> > .../bindings/spmi/qcom,spmi-pmic-arb.txt | 46 ++++++++++++++++++++++
> > 1 file changed, 46 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> >
> > diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> > new file mode 100644
> > index 0000000..9e50cb3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> > @@ -0,0 +1,46 @@
> > +Qualcomm SPMI Controller (PMIC Arbiter)
> > +
> > +The SPMI PMIC Arbiter is found on the Snapdragon 800 Series. It is an SPMI
> > +controller with wrapping arbitration logic to allow for multiple on-chip
> > +devices to control a single SPMI master.
> > +
> > +The PMIC Arbiter can also act as an interrupt controller, providing interrupts
> > +to slave devices.
> > +
> > +See spmi.txt for the generic SPMI controller binding requirements for child
> > +nodes.
> > +
> > +Required properties:
> > +- compatible : should be "qcom,spmi-pmic-arb".
> > +- reg-names : should be "core", "intr", "cnfg"
> > +- reg : register specifiers, must contain 3 entries, in the follow order:
> > + core registers, interrupt controller registers, configuration registers
>
> As far as I can tell, patch 3/6 doesn't require these to be in any
> order, as it uses 'reg-names' to fetch the values. Perhaps the
> following:
>
> reg-names : must contain:
> "core" - core registers
> "intr" - interrupt controller registers
> "cnfg" - configuration registers
> reg : A list of address + size pairs for the regs listed in reg-names
Yes, I think this reasonable. Thanks.
> > +- #address-cells : must be set to 2
> > +- #size-cells : must be set to 0
> > +- qcom,ee : indicates the active Execution Environment identifier (0-5)
> > +- qcom,channel : which of the PMIC Arb provided channels to use for accesses (0-5)
>
> These two are new.... Is it expected that they should be required?
The 'qcom,channel' property is unconditionally required. Previous
iterations of the patchset assumed the desired channel was 0, however,
this assumption does not hold for other on-SoC masters.
The 'qcom,ee' property is required for the same reason, however it's
only technically required if the PMIC Arb's interrupt functionality is
to be used. Right now the driver enforces this property's existence,
however technically this could be dropped to "optional". But, I imagine
nearly everyone using the SPMI PMIC arb will also want interrupts.
> > +- interrupt-controller : indicates the PMIC arbiter is an interrupt controller
>
> Although it's probably fairly understood that this is a boolean, it
> wouldn't hurt to mention that here. It might also be worth referencing
> devicetree/binding/interrupt-controller/interrupts.txt in your opening
> blurb.
Okay.
> > +- #interrupt-cells = <4>: interrupts are specified as a 4-tuple:
>
> Nitpick: This '= <X>' differs from the above 'must be set to X' format.
Indeed. Thanks.
> > + cell 1: slave ID for the requested interrupt (0-15)
> > + cell 2: peripheral ID for requested interrupt (0-255)
> > + cell 3: the requested peripheral interrupt (0-7)
> > + cell 4: interrupt flags indicating level-sense information, as defined in
> > + dt-bindings/interrupt-controller/irq.h
> [...]
>
> -Courtney
Thanks for the comments!
Josh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH v4 2/4] pinctrl: Add pinctrl binding for Broadcom Capri SoCs
From: Mark Rutland @ 2014-01-15 16:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdYCmMguUPFRAUHf186_ciKcchBZLGfgqn_VcSMiZi+a8Q@mail.gmail.com>
Hi Linus,
On Wed, Jan 15, 2014 at 09:40:53AM +0000, Linus Walleij wrote:
> On Tue, Jan 14, 2014 at 8:00 PM, Sherman Yin <syin@broadcom.com> wrote:
>
> > Great! Is there anything else you would like to see changed before this
> > patchset can be accepted?
>
> I'd like some sign of life from the DT binding maintainers.
Let's check the electrocardiogram...
_ _ _ _ _
__________/ \ ________/ \ _____/ \ ___/ \ ___/ \ __
\_/ \_/ \_/ \_/ \_/
I don't see anything objectionable in the binding.
Linus, if you think that this binding is sane, feel free to add my ack.
Cheers,
Mark.
^ permalink raw reply
* [PATCH v4 04/16] ARM: move generic thumb instruction parsing code to new files for use by other feature
From: David Long @ 2014-01-15 16:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1387543562.3404.35.camel@linaro1.home>
On 12/20/13 07:46, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Move the thumb version of the kprobes instruction parsing code into more generic
>> files from where it can be used by uprobes and possibly other subsystems. The
>> symbol names will be made more generic in a subsequent part of this patchset.
>>
>> Signed-off-by: David A. Long <dave.long@linaro.org>
>> ---
>
> I just have two comments about probes-thumb.h ...
>
> [...]
>
>> diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
>> new file mode 100644
>> index 0000000..3f39210
>> --- /dev/null
>> +++ b/arch/arm/kernel/probes-thumb.h
>> @@ -0,0 +1,136 @@
>> +/*
>> + * arch/arm/kernel/probes-arm.h
>> + *
>> + * Copyright 2013 Linaro Ltd.
>> + * Written by: David A. Long
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +#ifndef _ARM_KERNEL_PROBES_THUMB_H
>> +#define _ARM_KERNEL_PROBES_THUMB_H
>> +
>> +/*
>> + * True if current instruction is in an IT block.
>> + */
>> +#define in_it_block(cpsr) ((cpsr & 0x06000c00) != 0x00000000)
>> +
>> +/*
>> + * Return the condition code to check for the currently executing instruction.
>> + * This is in ITSTATE<7:4> which is in CPSR<15:12> but is only valid if
>> + * in_it_block returns true.
>> + */
>> +#define current_cond(cpsr) ((cpsr >> 12) & 0xf)
>
> Looks like you forgot to remove above two #defines from kprobes-thumb.c
> when you moved them to this header file.
Fixed.
> Also...
>
>> +enum probes_t32_action {
>> + PROBES_T32_EMULATE_NONE,
>> + PROBES_T32_SIMULATE_NOP,
>> + PROBES_T32_LDMSTM,
>> + PROBES_T32_LDRDSTRD,
>> + PROBES_T32_TABLE_BRANCH,
>> + PROBES_T32_TST,
>> + PROBES_T32_CMP,
>> + PROBES_T32_MOV,
>> + PROBES_T32_ADDSUB,
>> + PROBES_T32_LOGICAL,
>> + PROBES_T32_ADDWSUBW_PC,
>> + PROBES_T32_ADDWSUBW,
>> + PROBES_T32_MOVW,
>> + PROBES_T32_SAT,
>> + PROBES_T32_BITFIELD,
>> + PROBES_T32_SEV,
>> + PROBES_T32_WFE,
>> + PROBES_T32_MRS,
>> + PROBES_T32_BRANCH_COND,
>> + PROBES_T32_BRANCH,
>> + PROBES_T32_PLDI,
>> + PROBES_T32_LDR_LIT,
>> + PROBES_T32_LDRSTR,
>> + PROBES_T32_SIGN_EXTEND,
>> + PROBES_T32_MEDIA,
>> + PROBES_T32_REVERSE,
>> + PROBES_T32_MUL_ADD,
>> + PROBES_T32_MUL_ADD2,
>> + PROBES_T32_MUL_ADD_LONG
>> +};
>> +
>> +enum probes_t16_action {
>> + PROBES_T16_ADD_SP,
>> + PROBES_T16_CBZ,
>> + PROBES_T16_SIGN_EXTEND,
>> + PROBES_T16_PUSH,
>> + PROBES_T16_POP,
>> + PROBES_T16_SEV,
>> + PROBES_T16_WFE,
>> + PROBES_T16_IT,
>> + PROBES_T16_CMP,
>> + PROBES_T16_ADDSUB,
>> + PROBES_T16_LOGICAL,
>> + PROBES_T16_BLX,
>> + PROBES_T16_HIREGOPS,
>> + PROBES_T16_LDR_LIT,
>> + PROBES_T16_LDRHSTRH,
>> + PROBES_T16_LDRSTR,
>> + PROBES_T16_ADR,
>> + PROBES_T16_LDMSTM,
>> + PROBES_T16_BRANCH_COND,
>> + PROBES_T16_BRANCH
>> +};
>> +
>
Moved.
-dl
^ permalink raw reply
* [PATCH v4 02/16] ARM: move shared uprobe/kprobe definitions into new include file
From: David Long @ 2014-01-15 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1387543568.3404.36.camel@linaro1.home>
On 12/20/13 07:46, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Separate the kprobe-only definitions from the definitions needed by
>> both kprobes and uprobes.
>>
>> Signed-off-by: David A. Long <dave.long@linaro.org>
>> ---
>> arch/arm/include/asm/kprobes.h | 15 +--------------
>> arch/arm/include/asm/probes.h | 18 ++++++++++++++++++
>> 2 files changed, 19 insertions(+), 14 deletions(-)
>> create mode 100644 arch/arm/include/asm/probes.h
>>
>> diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
>> index f82ec22..30fc11b 100644
>> --- a/arch/arm/include/asm/kprobes.h
>> +++ b/arch/arm/include/asm/kprobes.h
>> @@ -28,21 +28,8 @@
>> #define kretprobe_blacklist_size 0
>>
>> typedef u32 kprobe_opcode_t;
>> -
>> struct kprobe;
>> -typedef void (kprobe_insn_handler_t)(struct kprobe *, struct pt_regs *);
>> -typedef unsigned long (kprobe_check_cc)(unsigned long);
>> -typedef void (kprobe_insn_singlestep_t)(struct kprobe *, struct pt_regs *);
>> -typedef void (kprobe_insn_fn_t)(void);
>> -
>> -/* Architecture specific copy of original instruction. */
>> -struct arch_specific_insn {
>> - kprobe_opcode_t *insn;
>> - kprobe_insn_handler_t *insn_handler;
>> - kprobe_check_cc *insn_check_cc;
>> - kprobe_insn_singlestep_t *insn_singlestep;
>> - kprobe_insn_fn_t *insn_fn;
>> -};
>> +#include <asm/probes.h>
>>
>> struct prev_kprobe {
>> struct kprobe *kp;
>> diff --git a/arch/arm/include/asm/probes.h b/arch/arm/include/asm/probes.h
>> new file mode 100644
>> index 0000000..21da148
>> --- /dev/null
>> +++ b/arch/arm/include/asm/probes.h
>> @@ -0,0 +1,18 @@
>
> This new file doesn't have any copyright/license notice, I suggest you
> copy the one from kprobes.h and include a note to say this new files
> contents were copied, e.g. start it like...
>
> /*
> * arch/arm/include/asm/probes.h
> *
> * Original contents copied from arch/arm/include/asm/kprobes.h
> * which contains the following notice...
> *
> * Copyright (C) 2006, 2007 Motorola Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> [...]
>
Fixed.
>> +#ifndef _ASM_PROBES_H
>> +#define _ASM_PROBES_H
>> +
>> +typedef void (kprobe_insn_handler_t)(struct kprobe *, struct pt_regs *);
>> +typedef unsigned long (kprobe_check_cc)(unsigned long);
>> +typedef void (kprobe_insn_singlestep_t)(struct kprobe *, struct pt_regs *);
>> +typedef void (kprobe_insn_fn_t)(void);
>> +
>> +/* Architecture specific copy of original instruction. */
>> +struct arch_specific_insn {
>> + kprobe_opcode_t *insn;
>> + kprobe_insn_handler_t *insn_handler;
>> + kprobe_check_cc *insn_check_cc;
>> + kprobe_insn_singlestep_t *insn_singlestep;
>> + kprobe_insn_fn_t *insn_fn;
>> +};
>> +
>> +#endif
>
-dl
^ permalink raw reply
* [PATCH v4 07/16] ARM: Remove use of struct kprobe from generic probes code
From: David Long @ 2014-01-15 16:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1387547729.3404.50.camel@linaro1.home>
On 12/20/13 08:55, Jon Medhurst (Tixy) wrote:
> On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Change the generic ARM probes code to pass in the opcode and architecture-specific
>> structure separately instead of using struct kprobe, so we do not pollute
>> code being used only for uprobes or other non-kprobes instruction
>> interpretation.
>>
>> Signed-off-by: David A. Long <dave.long@linaro.org>
>> ---
>> arch/arm/include/asm/probes.h | 9 +-
>> arch/arm/kernel/kprobes-arm.c | 77 ++++++-------
>> arch/arm/kernel/kprobes-common.c | 42 ++++---
>> arch/arm/kernel/kprobes-thumb.c | 239 +++++++++++++++++++--------------------
>> arch/arm/kernel/kprobes.c | 2 +-
>> arch/arm/kernel/kprobes.h | 2 +-
>> arch/arm/kernel/probes-arm.c | 33 +++---
>> arch/arm/kernel/probes-arm.h | 29 ++---
>> arch/arm/kernel/probes-thumb.c | 18 ++-
>> arch/arm/kernel/probes-thumb.h | 51 ---------
>> arch/arm/kernel/probes.c | 14 ++-
>> arch/arm/kernel/probes.h | 8 +-
>> 12 files changed, 246 insertions(+), 278 deletions(-)
>
> Looks OK to me, though it looks like this patch includes some header
> file cleanups that should have been part of the previous patch? I.e. all
> the changes to probes-thumb.h and probes-arm.h. And also the
> re-application of 'static' to functions in kprobes-thumb.c and
> kprobes-arm.c.
>
> I'm not sure it's worth the hassle of trying to unpick those changes
> though, it doesn't impact the end result or bisect-bility.
>
I moved the changes around as suggested.
-dl
^ permalink raw reply
* [PATCH v6 3/3] KGDB: make kgdb_breakpoint() as noinline
From: Jason Wessel @ 2014-01-15 16:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1387453851-19341-4-git-send-email-vijay.kilari@gmail.com>
On 12/19/2013 05:50 AM, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> The function kgdb_breakpoint() sets up break point at
> compile time by calling arch_kgdb_breakpoint();
> Though this call is surrounded by wmb() barrier,
> the compile can still re-order the break point,
> because this scheduling barrier is not a code motion
> barrier in gcc.
>
> Making kgdb_breakpoint() as noinline solves this problem
> of code reording around break point instruction and also
> avoids problem of being called as inline function from
> other places
>
> More details about discussion on this can be found here
> http://comments.gmane.org/gmane.linux.ports.arm.kernel/269732
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Jason Wessel <jason.wessel@windriver.com>
Cheers,
Jason.
> ---
> kernel/debug/debug_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 7d2f35e..cf04798 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -1034,7 +1034,7 @@ int dbg_io_get_char(void)
> * otherwise as a quick means to stop program execution and "break" into
> * the debugger.
> */
> -void kgdb_breakpoint(void)
> +noinline void kgdb_breakpoint(void)
> {
> atomic_inc(&kgdb_setting_breakpoint);
> wmb(); /* Sync point before breakpoint */
^ permalink raw reply
* [PATCH v2 2/2] i2c: New bus driver for the QUP I2C controller
From: Stephen Boyd @ 2014-01-15 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389659437-16406-3-git-send-email-bjorn.andersson@sonymobile.com>
On 01/13, Bjorn Andersson wrote:
> +/*
> + * QUP driver for Qualcomm MSM platforms
> + *
> + */
This comment seems redundant, we know what file we're looking at.
> +
> +struct qup_i2c_dev {
> + struct device *dev;
> + void __iomem *base;
> + struct pinctrl *pctrl;
This is unused.
> + int irq;
> + struct clk *clk;
> + struct clk *pclk;
> + struct i2c_adapter adap;
> +
> + int clk_freq;
This is only ever used in probe, so why do we need to store it
away?
> + int clk_ctl;
> + int one_bit_t;
> + int out_fifo_sz;
> + int in_fifo_sz;
> + int out_blk_sz;
> + int in_blk_sz;
> + unsigned long xfer_time;
> + unsigned long wait_idle;
> +
> + struct i2c_msg *msg;
> + /* Current possion in user message buffer */
s/possion/position/?
> + int pos;
> + /* Keep number of bytes left to be transmitted */
> + int cnt;
> + /* I2C protocol errors */
> + u32 bus_err;
> + /* QUP core errors */
> + u32 qup_err;
> + /*
> + * maximum bytes that could be send (per iterration). could be
s/iterration/iteration/?
> + * equal of fifo size or block size (in block mode)
> + */
> + int chunk_sz;
> + struct completion xfer;
> +};
> +
> +static irqreturn_t qup_i2c_interrupt(int irq, void *dev)
> +{
> + struct qup_i2c_dev *qup = dev;
> + u32 bus_err;
> + u32 qup_err;
> + u32 opflags;
> +
[...]
> +
> + if (opflags & QUP_OUT_SVC_FLAG)
> + writel(QUP_OUT_SVC_FLAG, qup->base + QUP_OPERATIONAL);
> +
> + if (!(qup->msg->flags == I2C_M_RD))
Should this be?
if (!(qup->msg->flags & I2C_M_RD))
Otherwise it should be
if (qup->msg->flags != I2C_M_RD)
> + return IRQ_HANDLED;
> +
> + if ((opflags & QUP_MX_INPUT_DONE) || (opflags & QUP_IN_SVC_FLAG))
> + writel(QUP_IN_SVC_FLAG, qup->base + QUP_OPERATIONAL);
> +
> +done:
> + qup->qup_err = qup_err;
> + qup->bus_err = bus_err;
> + complete(&qup->xfer);
> + return IRQ_HANDLED;
> +}
> +
> +static int
> +qup_i2c_poll_state(struct qup_i2c_dev *qup, u32 req_state, bool only_valid)
> +{
> + int retries = 0;
> + u32 state;
> +
> + do {
> + state = readl(qup->base + QUP_STATE);
> +
> + /*
> + * If only valid bit needs to be checked, requested state is
> + * 'don't care'
> + */
It looks like req_state == 0 means only_valid == true. Can we
drop the only_valid argument to this function?
> + if (state & QUP_STATE_VALID) {
> + if (only_valid)
> + return 0;
> + if ((req_state & QUP_I2C_MAST_GEN)
> + && (state & QUP_I2C_MAST_GEN))
> + return 0;
> + if ((state & QUP_STATE_MASK) == req_state)
> + return 0;
> + }
> +
> + if (retries++ == 1000)
> + udelay(100);
> +
> + } while (retries != 2000);
Please makes #defines for 1000 and 2000.
> +
> + return -ETIMEDOUT;
> +}
> +
[...]
> +static void qup_i2c_issue_write(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> +{
> + u32 addr = msg->addr << 1;
> + u32 val, qup_tag;
> + int idx, entries;
> +
> + if (qup->pos == 0) {
> + val = QUP_OUT_START | addr;
> + } else {
> + /*
> + * Avoid setup time issue by adding 1 NOP when number of bytes
> + * are more than FIFO/BLOCK size. setup time issue can't appear
> + * otherwise since next byte to be written will always be ready
> + */
> + val = (QUP_OUT_NOP | 1);
> + }
> +
> + entries = qup->cnt + 1;
> +
> + if (entries > qup->chunk_sz)
> + entries = qup->chunk_sz;
> +
> + qup_tag = QUP_OUT_DATA;
> +
> + /* Reserve one entry for STOP */
> + for (idx = 1; idx < (entries - 1); idx++, qup->pos++) {
Unnecessary () here around entries.
> +
> + if (idx & 1) {
> + val |= (qup_tag | msg->buf[qup->pos]) << QUP_MSW_SHIFT;
> + writel(val, qup->base + QUP_OUT_FIFO_BASE);
> + } else {
> + val = qup_tag | msg->buf[qup->pos];
> + }
[...]
> +
> +#ifdef CONFIG_PM
This ifdef is probably wrong considering that you can disable
CONFIG_PM_RUNTIME without disabling CONFIG_PM and then these
runtime PM functions would be unused.
> +static int qup_i2c_pm_suspend_runtime(struct device *device)
> +{
> + struct qup_i2c_dev *qup = dev_get_drvdata(device);
> +
> + dev_dbg(device, "pm_runtime: suspending...\n");
> + qup_i2c_disable_clocks(qup);
> + return 0;
> +}
> +
> +static int qup_i2c_pm_resume_runtime(struct device *device)
> +{
> + struct qup_i2c_dev *qup = dev_get_drvdata(device);
> +
> + dev_dbg(device, "pm_runtime: resuming...\n");
> + qup_i2c_enable_clocks(qup);
> + return 0;
> +}
> +
> +static int qup_i2c_suspend(struct device *device)
> +{
> + dev_dbg(device, "system suspend");
> + qup_i2c_pm_suspend_runtime(device);
> + return 0;
> +}
> +
> +static int qup_i2c_resume(struct device *device)
> +{
> + dev_dbg(device, "system resume");
> + qup_i2c_pm_resume_runtime(device);
> + pm_runtime_mark_last_busy(device);
> + pm_request_autosuspend(device);
> + return 0;
> +}
> +#endif /* CONFIG_PM */
> +
> +static const struct dev_pm_ops qup_i2c_qup_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(
> + qup_i2c_suspend,
> + qup_i2c_resume)
> + SET_RUNTIME_PM_OPS(
> + qup_i2c_pm_suspend_runtime,
> + qup_i2c_pm_resume_runtime,
> + NULL)
> +};
> +
> +static struct of_device_id qup_i2c_dt_match[] = {
const?
> + {.compatible = "qcom,i2c-qup"},
> + {}
> +};
MODULE_DEVICE_TABLE(of, qup_i2c_dt_match)?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH v6 3/3] KGDB: make kgdb_breakpoint() as noinline
From: Will Deacon @ 2014-01-15 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52D6BAEB.3040607@windriver.com>
On Wed, Jan 15, 2014 at 04:44:27PM +0000, Jason Wessel wrote:
> On 12/19/2013 05:50 AM, vijay.kilari at gmail.com wrote:
> > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >
> > The function kgdb_breakpoint() sets up break point at
> > compile time by calling arch_kgdb_breakpoint();
> > Though this call is surrounded by wmb() barrier,
> > the compile can still re-order the break point,
> > because this scheduling barrier is not a code motion
> > barrier in gcc.
> >
> > Making kgdb_breakpoint() as noinline solves this problem
> > of code reording around break point instruction and also
> > avoids problem of being called as inline function from
> > other places
> >
> > More details about discussion on this can be found here
> > http://comments.gmane.org/gmane.linux.ports.arm.kernel/269732
> >
> > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> > Acked-by: Will Deacon <will.deacon@arm.com>
>
> Acked-by: Jason Wessel <jason.wessel@windriver.com>
Thanks Jason! We'll take this series via the arm64 tree.
Will
^ permalink raw reply
* [PATCH v5 2/4] devicetree: bindings: Document Krait CPU/L1 EDAC
From: Stephen Boyd @ 2014-01-15 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140115102701.GA27314@e102568-lin.cambridge.arm.com>
On 01/15, Lorenzo Pieralisi wrote:
> On Tue, Jan 14, 2014 at 09:30:32PM +0000, Stephen Boyd wrote:
> > The Krait CPU/L1 error reporting device is made up a per-CPU
> > interrupt. While we're here, document the next-level-cache
> > property that's used by the Krait EDAC driver.
> >
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Kumar Gala <galak@codeaurora.org>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > ---
> > Documentation/devicetree/bindings/arm/cpus.txt | 52 ++++++++++++++++++++++++++
> > 1 file changed, 52 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> > index 91304353eea4..c332b5168456 100644
> > --- a/Documentation/devicetree/bindings/arm/cpus.txt
> > +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> > @@ -191,6 +191,16 @@ nodes to be present and contain the properties described below.
> > property identifying a 64-bit zero-initialised
> > memory location.
> >
> > + - interrupts
> > + Usage: required for cpus with compatible string "qcom,krait".
> > + Value type: <prop-encoded-array>
> > + Definition: L1/CPU error interrupt
>
> I reckon you want this property to belong in the cpus node (example below),
> not in cpu nodes, right ?
Yes.
>
> Are you relying on a platform device to be created for /cpus node in
> order for this series to work ? I guess that's why you want the
> interrupts property to be defined in /cpus so that it becomes a platform
> device resource (and you also add a compatible property in /cpus that is
> missing in these bindings).
Ah yes. I'll move this to the /cpus section.
>
> > +
> > + - next-level-cache
> > + Usage: optional
> > + Value type: <phandle>
> > + Definition: phandle pointing to the next level cache
> > +
> > Example 1 (dual-cluster big.LITTLE system 32-bit):
> >
> > cpus {
> > @@ -382,3 +392,45 @@ cpus {
> > cpu-release-addr = <0 0x20000000>;
> > };
> > };
> > +
> > +
> > +Example 5 (Krait 32-bit system):
> > +
> > +cpus {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + interrupts = <1 9 0xf04>;
>
> In patch 4 you also add a compatible property here, and that's not documented,
> and honestly I do not think that's acceptable either. I guess you want a
> compatible property here to match the platform driver, right ?
Ah sorry, I forgot to put the compatible property here like in
the dts change. I'll do that in the next revision. Yes we need a
compatible property here to match the platform driver.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH v3 4/7] ARM: brcmstb: add CPU binding for Broadcom Brahma15
From: Mark Rutland @ 2014-01-15 16:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389743333-16741-5-git-send-email-marc.ceeeee@gmail.com>
On Tue, Jan 14, 2014 at 11:48:50PM +0000, Marc Carino wrote:
> Add the Broadcom Brahma B15 CPU to the DT CPU binding list.
>
> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/devicetree/bindings/arm/cpus.txt | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 9130435..423b879 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -163,6 +163,7 @@ nodes to be present and contain the properties described below.
> "arm,cortex-r4"
> "arm,cortex-r5"
> "arm,cortex-r7"
> + "brcm,brahma15"
Given all the documentation I can find online calls the CPU
"Brahma-B15", would it not make more sense for this to be
"brcm,brahma-b15"?
That would match what we do for the ARM Cortex-[ar]\d processors.
Otherwise, this looks fine to me.
Cheers,
Mark.
^ permalink raw reply
* [RFC PATCH 3/9] of: mtd: add NAND timings retrieval support
From: boris brezillon @ 2014-01-15 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52D6A48D.4080302@overkiz.com>
On 15/01/2014 16:09, boris brezillon wrote:
> Hello Jason,
>
> On 09/01/2014 18:35, Jason Gunthorpe wrote:
>> On Thu, Jan 09, 2014 at 09:36:18AM +0100, boris brezillon wrote:
>>
>>>> You might want to check if you can boil down the DT timings from the
>>>> huge list to just an ONFI mode number..
>>> Sure, but the sunxi driver needs at least 19 of them...
>> So does mvebu's NAND driver..
>>
>> What I ment was you could have a
>>
>> onfi,nand-timing-mode = 0
>>
>> in the DT. Each of the modes defines all ~19 parameters, higher modes
>> are faster.
>>
>> Pick a mode value that fits all the parameters of the connected
>> non-ONFI flash.
>>
>> This would be instead of defining each parameter
>> individually.. Provide some helpers to convert from a onfi mode number
>> to all the onfi defined timing parameters so that drivers can
>> configure the HW..
>
> Are you suggesting we should provide a function that converts these
> modes into a nand_timings struct, or just use the timing modes and
> let the NAND controller drivers configure its IP accordingly ?
>
> I found the ONFI timing tables in this document:
>
> www.*onfi*.org/~/media/*ONFI*/specs/*onfi*_3_1_spec.pdf? (chapter 4.16).
>
> I suppose my nand_timings struct should use the names described
> page 110-111 (at least if we decide to use nand_timings and not
> nand_timing_modes), right ?
After taking a closer look at this document, the only parameter
available in my
nand_timings struct that is not defined in the standard is tR_max (data
transfer
from cell to register).
And the ONFI standard defines 4 more timings:
- tCEA_max
- tCEH_min
- tFEAT_max
- tITC_max
>
> Best Regards,
>
> Boris
>
>>
>> Jason
>
^ permalink raw reply
* [GIT PULL] SOCFPGA DT updates for 3.14 - take 2
From: dinguyen at altera.com @ 2014-01-15 17:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd, Kevin, and Olof,
Please consider pulling in this 1 patch for the SOCFPGA DT patch for v3.14.
Thanks,
Dinh
The following changes since commit 374b105797c3d4f29c685f3be535c35f5689b30e:
Linux 3.13-rc3 (2013-12-06 09:34:04 -0800)
are available in the git repository at:
git://git.rocketboards.org/linux-socfpga-next.git tags/socfpga-dt-for-3.14-take-2
for you to fetch changes up to 9cd20af9ccdf947a12518d36627dd46674784288:
dts: socfpga: Add support for SD/MMC on the SOCFPGA platform (2014-01-15 07:15:49 -0600)
----------------------------------------------------------------
SOCFPGA DT updates for v3.14 - take 2
----------------------------------------------------------------
Dinh Nguyen (1):
dts: socfpga: Add support for SD/MMC on the SOCFPGA platform
arch/arm/boot/dts/socfpga.dtsi | 13 ++++++++++++-
arch/arm/boot/dts/socfpga_arria5.dtsi | 11 +++++++++++
arch/arm/boot/dts/socfpga_cyclone5.dtsi | 11 +++++++++++
arch/arm/boot/dts/socfpga_vt.dts | 11 +++++++++++
4 files changed, 45 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH v4 4/6] spmi: pmic_arb: add support for interrupt handling
From: Josh Cartwright @ 2014-01-15 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140114234402.GD23276@sonymobile.com>
On Tue, Jan 14, 2014 at 03:44:03PM -0800, Courtney Cavin wrote:
> On Tue, Jan 14, 2014 at 07:41:38PM +0100, Josh Cartwright wrote:
> > The Qualcomm PMIC Arbiter, in addition to being a basic SPMI controller,
> > also implements interrupt handling for slave devices. Note, this is
> > outside the scope of SPMI, as SPMI leaves interrupt handling completely
> > unspecified.
> >
> > Extend the driver to provide a irq_chip implementation and chained irq
> > handling which allows for these interrupts to be used.
> >
> > Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> > ---
> > drivers/spmi/spmi-pmic-arb.c | 393 ++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 391 insertions(+), 2 deletions(-)
[..]
> > +struct spmi_pmic_arb_qpnpint_type {
> > + u8 type; /* 1 -> edge */
> > + u8 polarity_high;
> > + u8 polarity_low;
> > +} __packed;
> > +
>
> While the rest of this driver uses 'pmic' or 'spmi_pmic', this patch
> adds 'qpnpint'. Can we please just leave the software fabricated name
> 'qpnp' out of any changes, as it isn't in any hardware spec? Perhaps
> 'pmic_int' or something along those lines?
QPNP is not a software-created concept. It is a hardware concept, where
it places requirements on the layout of a peripherals' register space,
and how interrupts are expected to behave, among other things.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ 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