* [PATCH/RFT 00/12] Add DT support for ohci-da8xx
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
From: Axel Haslam <ahaslam@baylibre.com>
The purpose of this patch series is to add DT support to the ohci-da8xx
glue driver without breaking the non-DT boot, which is still used in
unconverted davinci devices.
To Achieve this, the first 8 patches make sure that the non-DT based
enumeration works, and prepares the stage for a DT migration by removing
dependencies on the board files (moving VBUS, and over current
handling to the driver). The last 4 patches actually add the DT
documentation and bindings for the ohci-da8xx driver.
Testing was done on a omap138-lcdk board, using DT, and non-DT boot,
and checking that in both cases the hub, usb mass storage and an input
device are correctly enumerated and working.
Since there have been some recent and ongoing efforts from David Lechner
to clean up davinci-mach code and the ochi-da8xx driver, this series
builds upon that work. Specifically:
* the accepted but soon to be reposted patch to remove mach code form
the ohci driver[1].
* The patch series to add phy nodes, and move usb clocks to a common
file [2].
A git branch based on tag: next-20161004 with the dependencies patches is
available in my github here [3].
The omap138-lcdk does not have gpios to control vbus and get over current
interrupt notifications, hence i was not able to test these and added
the RFT tag. If anyone has a da830-evm based board and could
confirm that ohci is correctly working, i would appreciate it.
(the OHCI option needs to be enabled in menuconfig)
P.D: It seems that the davinci-gpio driver is broken for DT based boot
and any gpio > 32. (luckly none of the DT based boards use gpios yet)
The probelm is that we have 144 gpios in the gpio controller as correctly
declared on the DT (they are not separate gpio controllers as in am3xx),
but the driver creates several gpio chips of 32 pins each, confusing the
"gpio chip to pin" matching logic of gpiolib-of. I think we might need to
fix this by creating a single gpio chip in gpio-davinci.c
[1] [PATCH v6 1/3] usb: ohci-da8xx: Remove code that references mach
http://www.gossamer-threads.com/lists/linux/kernel/2518807
[2] [PATCH v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
http://www.spinics.net/lists/linux-usb/msg140568.html
[3] github branch with all dependante patches
https://github.com/axelhaslamx/linux-axel/commits/ohci-da8xx-dt
Axel Haslam (12):
ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
ARM: davinci: hawk: add full constraints for ohci plat boot
ARM: davinci: rename root_hub to platform_data
USB: ohci-da8xx: Divide power up time in the ohci driver
USB: ohci-da8xx: Fix probe for devices with no vbus/oci gpio
ARM: davinci: hawk: Remove oci and vbus gpios
USB: ohci-da8xx: Request gpios and handle interrupt in the driver
ARM: davinci: register the usb20_phy clock on the SoC file
usb: host: ohci-da8xx: Add devicetree bindings documentation
USB: ohci-da8xx: Add device tree support
ARM: dts: da850: Add the usb ohci device node
ARM: dts: da850-lcdk: enable ohci usb
.../devicetree/bindings/usb/ohci-da8xx.txt | 32 +++++
arch/arm/boot/dts/da850-lcdk.dts | 9 ++
arch/arm/boot/dts/da850.dtsi | 8 ++
arch/arm/mach-davinci/board-da830-evm.c | 75 +---------
arch/arm/mach-davinci/board-omapl138-hawk.c | 105 +-------------
arch/arm/mach-davinci/da850.c | 2 +
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +-
arch/arm/mach-davinci/usb-da8xx.c | 15 +-
drivers/usb/host/ohci-da8xx.c | 158 +++++++++++++++++----
include/linux/platform_data/usb-davinci.h | 22 ++-
10 files changed, 218 insertions(+), 210 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
--
2.7.1
^ permalink raw reply
* [PATCH/RFT 01/12] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
While probing ochi phy with usb20 phy as a parent clock for usb11_phy,
the usb20_phy clock enable would time out. This is because the usb20
module clock needs to enabled while trying to lock the usb20_phy PLL.
Call clk enable and get for the usb20 peripheral before trying to
enable the phy PLL.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/usb-da8xx.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index 9e41a7f..982e105 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -53,11 +53,19 @@ int __init da8xx_register_usb_refclkin(int rate)
static void usb20_phy_clk_enable(struct clk *clk)
{
+ struct clk *usb20_clk;
u32 val;
u32 timeout = 500000; /* 500 msec */
val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+ usb20_clk = clk_get(NULL, "usb20");
+ if (IS_ERR(usb20_clk)) {
+ pr_err("could not get usb20 clk\n");
+ return;
+ }
+
+ clk_prepare_enable(usb20_clk);
/*
* Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
* host may use the PLL clock without USB 2.0 OTG being used.
@@ -70,11 +78,14 @@ static void usb20_phy_clk_enable(struct clk *clk)
while (--timeout) {
val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
if (val & CFGCHIP2_PHYCLKGD)
- return;
+ goto done;
udelay(1);
}
pr_err("Timeout waiting for USB 2.0 PHY clock good.\n");
+done:
+ clk_disable_unprepare(usb20_clk);
+ clk_put(usb20_clk);
}
static void usb20_phy_clk_disable(struct clk *clk)
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 02/12] ARM: davinci: hawk: add full constraints for ohci plat boot
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
The phy framework requests an optional "phy" regulator. If it does
not find one, it returns -EPROBE_DEFER. In the case of non-DT based boot
for the omap138-lcdk board, this would prevent the usb11 phy to probe
correctly and ohci would not enumerate.
By calling "regulator_has_full_constraints", An error would be returned
instead of DEFER for the "optional" regulator, and the probe of
the phy driver can continue normally without a regulator.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-omapl138-hawk.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index c5cb8d9..1dc6112 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -14,6 +14,7 @@
#include <linux/console.h>
#include <linux/gpio.h>
#include <linux/platform_data/gpio-davinci.h>
+#include <linux/regulator/machine.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -328,6 +329,7 @@ static __init void omapl138_hawk_init(void)
if (ret)
pr_warn("%s: dsp/rproc registration failed: %d\n",
__func__, ret);
+ regulator_has_full_constraints();
}
#ifdef CONFIG_SERIAL_8250_CONSOLE
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 03/12] ARM: davinci: rename root_hub to platform_data
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
To prepare for DT support we will remove the usb callback function
pointers. Since this structure will only pass data to the driver, It
seems better to rename the structure root_hub structure to platform_data
There is no functional change.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 2 +-
arch/arm/mach-davinci/board-omapl138-hawk.c | 2 +-
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +-
arch/arm/mach-davinci/usb-da8xx.c | 2 +-
drivers/usb/host/ohci-da8xx.c | 36 ++++++++++++++---------------
include/linux/platform_data/usb-davinci.h | 6 ++---
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index c62766e..18d2b10 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -87,7 +87,7 @@ static int da830_evm_usb_ocic_notify(da8xx_ocic_handler_t handler)
return error;
}
-static struct da8xx_ohci_root_hub da830_evm_usb11_pdata = {
+static struct da8xx_ohci_platform_data da830_evm_usb11_pdata = {
.set_power = da830_evm_usb_set_power,
.get_power = da830_evm_usb_get_power,
.get_oci = da830_evm_usb_get_oci,
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 1dc6112..a2bf3eb 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -226,7 +226,7 @@ static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
return error;
}
-static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
+static struct da8xx_ohci_platform_data omapl138_hawk_usb11_pdata = {
.set_power = hawk_usb_set_power,
.get_power = hawk_usb_get_power,
.get_oci = hawk_usb_get_oci,
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 38d932e..f7d6fd9 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -94,7 +94,7 @@ int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
int da8xx_register_usb_phy(void);
int da8xx_register_usb20(unsigned mA, unsigned potpgt);
-int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
+int da8xx_register_usb11(struct da8xx_ohci_platform_data *pdata);
int da8xx_register_emac(void);
int da8xx_register_uio_pruss(void);
int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index 982e105..36e3460 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -337,7 +337,7 @@ static struct platform_device da8xx_usb11_device = {
.resource = da8xx_usb11_resources,
};
-int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata)
+int __init da8xx_register_usb11(struct da8xx_ohci_platform_data *pdata)
{
da8xx_usb11_device.dev.platform_data = pdata;
return platform_device_register(&da8xx_usb11_device);
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 3656d7c..8ed9a52 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -64,20 +64,20 @@ static void ohci_da8xx_disable(void)
/*
* Handle the port over-current indicator change.
*/
-static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
+static void ohci_da8xx_ocic_handler(struct da8xx_ohci_platform_data *pdata,
unsigned port)
{
ocic_mask |= 1 << port;
/* Once over-current is detected, the port needs to be powered down */
- if (hub->get_oci(port) > 0)
- hub->set_power(port, 0);
+ if (pdata->get_oci(port) > 0)
+ pdata->set_power(port, 0);
}
static int ohci_da8xx_init(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ struct da8xx_ohci_platform_data *pdata = dev_get_platdata(dev);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int result;
u32 rh_a;
@@ -107,16 +107,16 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
* the correct hub descriptor...
*/
rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (hub->set_power) {
+ if (pdata->set_power) {
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
}
- if (hub->get_oci) {
+ if (pdata->get_oci) {
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
}
rh_a &= ~RH_A_POTPGT;
- rh_a |= hub->potpgt << 24;
+ rh_a |= pdata->potpgt << 24;
ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
return result;
@@ -167,7 +167,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ struct da8xx_ohci_platform_data *pdata = dev_get_platdata(dev);
int temp;
switch (typeReq) {
@@ -181,11 +181,11 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (hub->get_power && hub->get_power(wIndex) == 0)
+ if (pdata->get_power && pdata->get_power(wIndex) == 0)
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (hub->get_oci && hub->get_oci(wIndex) > 0)
+ if (pdata->get_oci && pdata->get_oci(wIndex) > 0)
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
@@ -210,10 +210,10 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex, "POWER");
- if (!hub->set_power)
+ if (!pdata->set_power)
return -EPIPE;
- return hub->set_power(wIndex, temp) ? -EPIPE : 0;
+ return pdata->set_power(wIndex, temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
@@ -288,12 +288,12 @@ static const struct hc_driver ohci_da8xx_hc_driver = {
static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
struct platform_device *pdev)
{
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
+ struct da8xx_ohci_platform_data *pdata = dev_get_platdata(&pdev->dev);
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
- if (hub == NULL)
+ if (pdata == NULL)
return -ENODEV;
usb11_clk = devm_clk_get(&pdev->dev, "usb11");
@@ -337,8 +337,8 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
device_wakeup_enable(hcd->self.controller);
- if (hub->ocic_notify) {
- error = hub->ocic_notify(ohci_da8xx_ocic_handler);
+ if (pdata->ocic_notify) {
+ error = pdata->ocic_notify(ohci_da8xx_ocic_handler);
if (!error)
return 0;
}
@@ -361,9 +361,9 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
static inline void
usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev)
{
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
+ struct da8xx_ohci_platform_data *pdata = dev_get_platdata(&pdev->dev);
- hub->ocic_notify(NULL);
+ pdata->ocic_notify(NULL);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
}
diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h
index e0bc4ab..dffe3bf 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -34,13 +34,13 @@
#define CFGCHIP2_REFFREQ_24MHZ (2 << 0)
#define CFGCHIP2_REFFREQ_48MHZ (3 << 0)
-struct da8xx_ohci_root_hub;
+struct da8xx_ohci_platform_data;
-typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub,
+typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_platform_data *pdata,
unsigned port);
/* Passed as the platform data to the OHCI driver */
-struct da8xx_ohci_root_hub {
+struct da8xx_ohci_platform_data {
/* Switch the port power on/off */
int (*set_power)(unsigned port, int on);
/* Read the port power status */
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 04/12] USB: ohci-da8xx: Divide power up time in the ohci driver
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
Instead of requiring platform data to know that the effective time
should be diveded by two, Make that operation in the driver so that
users (platform board files/Device tree) dont have to worry about it.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 2 +-
arch/arm/mach-davinci/board-omapl138-hawk.c | 2 +-
drivers/usb/host/ohci-da8xx.c | 8 ++++++--
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 18d2b10..8d126e4 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -94,7 +94,7 @@ static struct da8xx_ohci_platform_data da830_evm_usb11_pdata = {
.ocic_notify = da830_evm_usb_ocic_notify,
/* TPS2065 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
+ .potpgt = 3, /* 3 ms max */
};
static irqreturn_t da830_evm_usb_ocic_irq(int irq, void *dev_id)
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index a2bf3eb..f9cd388 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -232,7 +232,7 @@ static struct da8xx_ohci_platform_data omapl138_hawk_usb11_pdata = {
.get_oci = hawk_usb_get_oci,
.ocic_notify = hawk_usb_ocic_notify,
/* TPS2087 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
+ .potpgt = 3 /* 3 ms max */
};
static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 8ed9a52..3c85d6c 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -115,8 +115,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
}
- rh_a &= ~RH_A_POTPGT;
- rh_a |= pdata->potpgt << 24;
+
+ if (pdata->potpgt) {
+ rh_a &= ~RH_A_POTPGT;
+ rh_a |= (DIV_ROUND_UP(pdata->potpgt, 2) << 24);
+ }
+
ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
return result;
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 05/12] USB: ohci-da8xx: Fix probe for devices with no vbus/oci gpio
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
Some boards dont have gpios assigened for vbus or oci.
Allow these boards to enumerate usb without declaring the set_power
and/or the ocic_notify callbacks in platform data.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 3c85d6c..9d9f8e3 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -215,7 +215,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp ? "Set" : "Clear", wIndex, "POWER");
if (!pdata->set_power)
- return -EPIPE;
+ return 0;
return pdata->set_power(wIndex, temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
@@ -343,10 +343,12 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
if (pdata->ocic_notify) {
error = pdata->ocic_notify(ohci_da8xx_ocic_handler);
- if (!error)
- return 0;
+ if (error)
+ goto err_notify;
}
+ return 0;
+err_notify:
usb_remove_hcd(hcd);
err:
usb_put_hcd(hcd);
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 06/12] ARM: davinci: hawk: Remove oci and vbus gpios
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
The omap138-lcdk and its predecessor the hawk board don't have gpios
connected to control vbus or get the over current notifications for usb.
for example, in the hawk board gpio6-13 is connected to a LED, and
gpio2-4 is not connected at all. In the lcdk board, gpio 2-4 is a push
button, and gpio6-13 is connected to a LED.
Remove the gpio and interrupt registration for these pins.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-omapl138-hawk.c | 78 +----------------------------
1 file changed, 1 insertion(+), 77 deletions(-)
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index f9cd388..075be1b 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -28,9 +28,6 @@
#define DA850_HAWK_MMCSD_CD_PIN GPIO_TO_PIN(3, 12)
#define DA850_HAWK_MMCSD_WP_PIN GPIO_TO_PIN(3, 13)
-#define DA850_USB1_VBUS_PIN GPIO_TO_PIN(2, 4)
-#define DA850_USB1_OC_PIN GPIO_TO_PIN(6, 13)
-
static short omapl138_hawk_mii_pins[] __initdata = {
DA850_MII_TXEN, DA850_MII_TXCLK, DA850_MII_COL, DA850_MII_TXD_3,
DA850_MII_TXD_2, DA850_MII_TXD_1, DA850_MII_TXD_0, DA850_MII_RXER,
@@ -181,66 +178,16 @@ static __init void omapl138_hawk_mmc_init(void)
gpio_free(DA850_HAWK_MMCSD_CD_PIN);
}
-static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id);
-static da8xx_ocic_handler_t hawk_usb_ocic_handler;
-
static const short da850_hawk_usb11_pins[] = {
DA850_GPIO2_4, DA850_GPIO6_13,
-1
};
-static int hawk_usb_set_power(unsigned port, int on)
-{
- gpio_set_value(DA850_USB1_VBUS_PIN, on);
- return 0;
-}
-
-static int hawk_usb_get_power(unsigned port)
-{
- return gpio_get_value(DA850_USB1_VBUS_PIN);
-}
-
-static int hawk_usb_get_oci(unsigned port)
-{
- return !gpio_get_value(DA850_USB1_OC_PIN);
-}
-
-static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
-{
- int irq = gpio_to_irq(DA850_USB1_OC_PIN);
- int error = 0;
-
- if (handler != NULL) {
- hawk_usb_ocic_handler = handler;
-
- error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
- IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING,
- "OHCI over-current indicator", NULL);
- if (error)
- pr_err("%s: could not request IRQ to watch "
- "over-current indicator changes\n", __func__);
- } else {
- free_irq(irq, NULL);
- }
- return error;
-}
-
static struct da8xx_ohci_platform_data omapl138_hawk_usb11_pdata = {
- .set_power = hawk_usb_set_power,
- .get_power = hawk_usb_get_power,
- .get_oci = hawk_usb_get_oci,
- .ocic_notify = hawk_usb_ocic_notify,
/* TPS2087 switch @ 5V */
.potpgt = 3 /* 3 ms max */
};
-static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id)
-{
- hawk_usb_ocic_handler(&omapl138_hawk_usb11_pdata, 1);
- return IRQ_HANDLED;
-}
-
static __init void omapl138_hawk_usb_init(void)
{
int ret;
@@ -266,34 +213,11 @@ static __init void omapl138_hawk_usb_init(void)
pr_warn("%s: USB PHY registration failed: %d\n",
__func__, ret);
- ret = gpio_request_one(DA850_USB1_VBUS_PIN,
- GPIOF_DIR_OUT, "USB1 VBUS");
- if (ret < 0) {
- pr_err("%s: failed to request GPIO for USB 1.1 port "
- "power control: %d\n", __func__, ret);
- return;
- }
-
- ret = gpio_request_one(DA850_USB1_OC_PIN,
- GPIOF_DIR_IN, "USB1 OC");
- if (ret < 0) {
- pr_err("%s: failed to request GPIO for USB 1.1 port "
- "over-current indicator: %d\n", __func__, ret);
- goto usb11_setup_oc_fail;
- }
-
ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
- if (ret) {
+ if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
- goto usb11_setup_fail;
- }
return;
-
-usb11_setup_fail:
- gpio_free(DA850_USB1_OC_PIN);
-usb11_setup_oc_fail:
- gpio_free(DA850_USB1_VBUS_PIN);
}
static __init void omapl138_hawk_init(void)
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 07/12] USB: ohci-da8xx: Request gpios and handle interrupt in the driver
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
Currently requesting the vbus and overcurrent gpio is handled on
the board specific file. But this does not play well moving to
device tree.
In preparation to migrate to a device tree boot, handle requesting
gpios and overcurrent interrupt on the usb driver itself, thus avoiding
callbacks to arch/mach*
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 71 ++---------------------
arch/arm/mach-davinci/board-omapl138-hawk.c | 11 ----
drivers/usb/host/ohci-da8xx.c | 90 +++++++++++++++++++++++------
include/linux/platform_data/usb-davinci.h | 16 +++--
4 files changed, 82 insertions(+), 106 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 8d126e4..cfba9fa 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -47,62 +47,15 @@ static const short da830_evm_usb11_pins[] = {
-1
};
-static da8xx_ocic_handler_t da830_evm_usb_ocic_handler;
-
-static int da830_evm_usb_set_power(unsigned port, int on)
-{
- gpio_set_value(ON_BD_USB_DRV, on);
- return 0;
-}
-
-static int da830_evm_usb_get_power(unsigned port)
-{
- return gpio_get_value(ON_BD_USB_DRV);
-}
-
-static int da830_evm_usb_get_oci(unsigned port)
-{
- return !gpio_get_value(ON_BD_USB_OVC);
-}
-
-static irqreturn_t da830_evm_usb_ocic_irq(int, void *);
-
-static int da830_evm_usb_ocic_notify(da8xx_ocic_handler_t handler)
-{
- int irq = gpio_to_irq(ON_BD_USB_OVC);
- int error = 0;
-
- if (handler != NULL) {
- da830_evm_usb_ocic_handler = handler;
-
- error = request_irq(irq, da830_evm_usb_ocic_irq,
- IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
- "OHCI over-current indicator", NULL);
- if (error)
- pr_err("%s: could not request IRQ to watch over-current indicator changes\n",
- __func__);
- } else
- free_irq(irq, NULL);
-
- return error;
-}
-
static struct da8xx_ohci_platform_data da830_evm_usb11_pdata = {
- .set_power = da830_evm_usb_set_power,
- .get_power = da830_evm_usb_get_power,
- .get_oci = da830_evm_usb_get_oci,
- .ocic_notify = da830_evm_usb_ocic_notify,
-
+ .gpio_vbus = ON_BD_USB_DRV,
+ .gpio_overcurrent = ON_BD_USB_OVC,
+ .flags = (DA8XX_OHCI_FLAG_GPIO_VBUS
+ | DA8XX_OHCI_FLAG_GPIO_OCI),
/* TPS2065 switch @ 5V */
.potpgt = 3, /* 3 ms max */
};
-static irqreturn_t da830_evm_usb_ocic_irq(int irq, void *dev_id)
-{
- da830_evm_usb_ocic_handler(&da830_evm_usb11_pdata, 1);
- return IRQ_HANDLED;
-}
-
static __init void da830_evm_usb_init(void)
{
int ret;
@@ -143,22 +96,6 @@ static __init void da830_evm_usb_init(void)
return;
}
- ret = gpio_request(ON_BD_USB_DRV, "ON_BD_USB_DRV");
- if (ret) {
- pr_err("%s: failed to request GPIO for USB 1.1 port power control: %d\n",
- __func__, ret);
- return;
- }
- gpio_direction_output(ON_BD_USB_DRV, 0);
-
- ret = gpio_request(ON_BD_USB_OVC, "ON_BD_USB_OVC");
- if (ret) {
- pr_err("%s: failed to request GPIO for USB 1.1 port over-current indicator: %d\n",
- __func__, ret);
- return;
- }
- gpio_direction_input(ON_BD_USB_OVC);
-
ret = da8xx_register_usb11(&da830_evm_usb11_pdata);
if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 075be1b..8d72bc1 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -178,11 +178,6 @@ static __init void omapl138_hawk_mmc_init(void)
gpio_free(DA850_HAWK_MMCSD_CD_PIN);
}
-static const short da850_hawk_usb11_pins[] = {
- DA850_GPIO2_4, DA850_GPIO6_13,
- -1
-};
-
static struct da8xx_ohci_platform_data omapl138_hawk_usb11_pdata = {
/* TPS2087 switch @ 5V */
.potpgt = 3 /* 3 ms max */
@@ -192,12 +187,6 @@ static __init void omapl138_hawk_usb_init(void)
{
int ret;
- ret = davinci_cfg_reg_list(da850_hawk_usb11_pins);
- if (ret) {
- pr_warn("%s: USB 1.1 PinMux setup failed: %d\n", __func__, ret);
- return;
- }
-
/* USB_REFCLKIN is not used. */
ret = da8xx_register_usb20_phy_clk(false);
if (ret)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 9d9f8e3..d7a0f11 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -17,6 +17,7 @@
#include <linux/clk.h>
#include <linux/phy/phy.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/gpio.h>
#ifndef CONFIG_ARCH_DAVINCI_DA8XX
#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX."
@@ -61,6 +62,24 @@ static void ohci_da8xx_disable(void)
clk_disable_unprepare(usb11_clk);
}
+
+static int ohci_da8xx_set_power(struct da8xx_ohci_platform_data *pdata,
+ int on)
+{
+ gpio_set_value(pdata->gpio_vbus, on);
+ return 0;
+}
+
+static int ohci_da8xx_get_power(struct da8xx_ohci_platform_data *pdata)
+{
+ return gpio_get_value(pdata->gpio_vbus);
+}
+
+static int ohci_da8xx_get_oci(struct da8xx_ohci_platform_data *pdata)
+{
+ return !gpio_get_value(pdata->gpio_overcurrent);
+}
+
/*
* Handle the port over-current indicator change.
*/
@@ -70,8 +89,18 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_platform_data *pdata,
ocic_mask |= 1 << port;
/* Once over-current is detected, the port needs to be powered down */
- if (pdata->get_oci(port) > 0)
- pdata->set_power(port, 0);
+ if (ohci_da8xx_get_oci(pdata) > 0)
+ ohci_da8xx_set_power(pdata, 0);
+}
+
+static irqreturn_t ohci_da8xx_ocic_irq(int irq, void *data)
+{
+ struct platform_device *pdev = (struct platform_device *) data;
+ struct da8xx_ohci_platform_data *pdata = dev_get_platdata(&pdev->dev);
+
+ ohci_da8xx_ocic_handler(pdata, 1);
+
+ return IRQ_HANDLED;
}
static int ohci_da8xx_init(struct usb_hcd *hcd)
@@ -107,11 +136,11 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
* the correct hub descriptor...
*/
rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (pdata->set_power) {
+ if (pdata->flags & DA8XX_OHCI_FLAG_GPIO_VBUS) {
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
}
- if (pdata->get_oci) {
+ if (pdata->flags & DA8XX_OHCI_FLAG_GPIO_OCI) {
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
}
@@ -185,11 +214,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (pdata->get_power && pdata->get_power(wIndex) == 0)
+ if ((pdata->flags & DA8XX_OHCI_FLAG_GPIO_VBUS)
+ && ohci_da8xx_get_power(pdata) == 0)
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (pdata->get_oci && pdata->get_oci(wIndex) > 0)
+ if ((pdata->flags & DA8XX_OHCI_FLAG_GPIO_OCI)
+ && ohci_da8xx_get_oci(pdata) > 0)
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
@@ -214,10 +245,10 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex, "POWER");
- if (!pdata->set_power)
+ if (!(pdata->flags & DA8XX_OHCI_FLAG_GPIO_VBUS))
return 0;
- return pdata->set_power(wIndex, temp) ? -EPIPE : 0;
+ return ohci_da8xx_set_power(pdata, temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
@@ -314,6 +345,38 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
return PTR_ERR(usb11_phy);
}
+
+ if (pdata->flags & DA8XX_OHCI_FLAG_GPIO_VBUS) {
+ error = devm_gpio_request_one(&pdev->dev,
+ pdata->gpio_vbus,
+ GPIOF_DIR_OUT, "usb11 vbus");
+ if (error) {
+ pr_err("could not request vbus gpio: %d\n", error);
+ return error;
+ }
+ }
+
+ if (pdata->flags & DA8XX_OHCI_FLAG_GPIO_OCI) {
+ error = devm_gpio_request_one(&pdev->dev,
+ pdata->gpio_overcurrent,
+ GPIOF_DIR_IN, "usb11 oci");
+ if (error) {
+ pr_err("could not request oci gpio: %d\n", error);
+ return error;
+ }
+
+ error = devm_request_irq(&pdev->dev,
+ gpio_to_irq(pdata->gpio_overcurrent),
+ ohci_da8xx_ocic_irq,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING,
+ "ohci overcurrent indicator", pdev);
+ if (error) {
+ pr_err("could not request oci irq: %d\n", error);
+ return error;
+ }
+ }
+
hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd)
return -ENOMEM;
@@ -341,15 +404,7 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
device_wakeup_enable(hcd->self.controller);
- if (pdata->ocic_notify) {
- error = pdata->ocic_notify(ohci_da8xx_ocic_handler);
- if (error)
- goto err_notify;
- }
-
return 0;
-err_notify:
- usb_remove_hcd(hcd);
err:
usb_put_hcd(hcd);
return error;
@@ -367,9 +422,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
static inline void
usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev)
{
- struct da8xx_ohci_platform_data *pdata = dev_get_platdata(&pdev->dev);
-
- pdata->ocic_notify(NULL);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
}
diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h
index dffe3bf..b72f703 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -41,17 +41,15 @@ typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_platform_data *pdata,
/* Passed as the platform data to the OHCI driver */
struct da8xx_ohci_platform_data {
- /* Switch the port power on/off */
- int (*set_power)(unsigned port, int on);
- /* Read the port power status */
- int (*get_power)(unsigned port);
- /* Read the port over-current indicator */
- int (*get_oci)(unsigned port);
- /* Over-current indicator change notification (pass NULL to disable) */
- int (*ocic_notify)(da8xx_ocic_handler_t handler);
-
/* Time from power on to power good (in 2 ms units) */
u8 potpgt;
+
+ u32 flags;
+#define DA8XX_OHCI_FLAG_GPIO_VBUS (1 << 0)
+#define DA8XX_OHCI_FLAG_GPIO_OCI (1 << 1)
+
+ int gpio_vbus;
+ int gpio_overcurrent;
};
void davinci_setup_usb(unsigned mA, unsigned potpgt_ms);
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 08/12] ARM: davinci: register the usb20_phy clock on the SoC file
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
The usb20_phy clock needs to be registered for the driver to be able
to get and enable a clock. Currently the usb phy clocks are registered
form board files, which will not be called during a device tree based
boot.
To be able to probe correctly usb form a device tree boot, register
the usb phy clocks form the SoC specific init.
Unfourtunatly, davinci does not have proper clock support on device tree
yet, so by registering the clock form de SoC specific file we are
forced to hardcode the parent clock, and cannot select refclkin as
parent for any of the phy clocks of the da850 family.
As none of the current da850 based boards currently in mainline use
refclkin as source. I guess we can live with this limitation until clocks
are correctly represented through CCF/device tree.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-omapl138-hawk.c | 10 ----------
arch/arm/mach-davinci/da850.c | 2 ++
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 8d72bc1..a78fa16 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -187,16 +187,6 @@ static __init void omapl138_hawk_usb_init(void)
{
int ret;
- /* USB_REFCLKIN is not used. */
- ret = da8xx_register_usb20_phy_clk(false);
- if (ret)
- pr_warn("%s: USB 2.0 PHY CLK registration failed: %d\n",
- __func__, ret);
- ret = da8xx_register_usb11_phy_clk(false);
- if (ret)
- pr_warn("%s: USB 1.1 PHY CLK registration failed: %d\n",
- __func__, ret);
-
ret = da8xx_register_usb_phy();
if (ret)
pr_warn("%s: USB PHY registration failed: %d\n",
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index ed3d0e9..621880d 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1350,4 +1350,6 @@ void __init da850_init(void)
__raw_writel(v, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG));
davinci_clk_init(davinci_soc_info_da850.cpu_clks);
+ da8xx_register_usb20_phy_clk(false);
+ da8xx_register_usb11_phy_clk(false);
}
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 09/12] usb: host: ohci-da8xx: Add devicetree bindings documentation
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
.../devicetree/bindings/usb/ohci-da8xx.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 0000000..e954ce5
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,32 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg: Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys: Phandle for the PHY device
+ - phy-names: Should be "usb-phy"
+
+Optional properties:
+ - vbus-gpio: If present, specifies a gpio that needs to be
+ activated for the bus to be powered.
+ - oci-gpio: If present, specifies a gpio that needs to be
+ activated for the overcurrent detection.
+ - power_on_delay: Power On to Power Good time - in ms.
+
+Example for omap138-lck:
+
+usb_phy: usb-phy {
+ compatible = "ti,da830-usb-phy";
+ #phy-cells = <1>;
+ status = "disabled";
+};
+usb11: usb11 at 0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ status = "disabled";
+};
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 10/12] USB: ohci-da8xx: Add device tree support
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This allows the controller to be specified via device tree.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 52 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index d7a0f11..10db421 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -18,6 +18,7 @@
#include <linux/phy/phy.h>
#include <linux/platform_data/usb-davinci.h>
#include <linux/gpio.h>
+#include <linux/of_gpio.h>
#ifndef CONFIG_ARCH_DAVINCI_DA8XX
#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX."
@@ -311,6 +312,47 @@ static const struct hc_driver ohci_da8xx_hc_driver = {
/*-------------------------------------------------------------------------*/
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_ohci_ids[] = {
+ { .compatible = "ti,da830-ohci" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
+
+static int ohci_da8xx_of_init(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct da8xx_ohci_platform_data *pdata;
+ u32 tmp;
+
+ if (!np)
+ return 0;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata->gpio_vbus = of_get_named_gpio(np, "vbus-gpio", 0);
+ if (pdata->gpio_vbus >= 0)
+ pdata->flags |= DA8XX_OHCI_FLAG_GPIO_VBUS;
+
+ pdata->gpio_overcurrent = of_get_named_gpio(np, "oci-gpio", 0);
+ if (pdata->gpio_overcurrent >= 0)
+ pdata->flags |= DA8XX_OHCI_FLAG_GPIO_OCI;
+
+ if (!of_property_read_u32(np, "power-on-delay", &tmp))
+ pdata->potpgt = tmp;
+
+ pdev->dev.platform_data = pdata;
+
+ return 0;
+}
+#else
+static int ohci_da8xx_of_init(struct platform_device *pdev)
+{
+ return 0;
+}
+#endif
/**
* usb_hcd_da8xx_probe - initialize DA8xx-based HCDs
@@ -323,12 +365,17 @@ static const struct hc_driver ohci_da8xx_hc_driver = {
static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
struct platform_device *pdev)
{
- struct da8xx_ohci_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct da8xx_ohci_platform_data *pdata;
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
- if (pdata == NULL)
+ error = ohci_da8xx_of_init(pdev);
+ if (error)
+ pr_err("error initializing platform data: %d\n", error);
+
+ pdata = dev_get_platdata(&pdev->dev);
+ if (!pdata)
return -ENODEV;
usb11_clk = devm_clk_get(&pdev->dev, "usb11");
@@ -498,6 +545,7 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
#endif
.driver = {
.name = "ohci",
+ .of_match_table = da8xx_ohci_ids,
},
};
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 11/12] ARM: dts: da850: Add the usb ohci device node
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This adds the device tree node for the usb11 (ohci)
controller present in the da850 family of SoC's.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/boot/dts/da850.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 33fcdce..afae565 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -381,6 +381,14 @@
#phy-cells = <1>;
status = "disabled";
};
+ usb11: usb11 at 0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ status = "disabled";
+ };
gpio: gpio at 226000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
--
2.7.1
^ permalink raw reply related
* [PATCH/RFT 12/12] ARM: dts: da850-lcdk: enable ohci usb
From: ahaslam at baylibre.com @ 2016-10-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-1-git-send-email-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This enables the ohci usb controller for the lcdk board.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..6da9d843 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -86,6 +86,15 @@
};
};
+&usb_phy {
+ status = "okay";
+};
+
+&usb11 {
+ status = "okay";
+ power-on-delay = <3>;
+};
+
&serial2 {
pinctrl-names = "default";
pinctrl-0 = <&serial2_rxtx_pins>;
--
2.7.1
^ permalink raw reply related
* [PATCH 1/2] ARM: imx: fix integer overflow in AV PLL round rate
From: Fabio Estevam @ 2016-10-07 16:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007160231.GA10773@workstation.local>
On Fri, Oct 7, 2016 at 1:02 PM, Emil Lundmark <emil@limesaudio.com> wrote:
> I was not aware of that issue before but it seems related. Only the first
> patch in the series is relevant for solving the issue I was experiencing.
Also, in v2 please describe exactly the issue you were experiencing.
Is the issue you observed a regression caused by ba7f4f557eb6 ("clk:
imx: correct AV PLL rate formula")?
If so, please state this in the commit log.
^ permalink raw reply
* [PATCH v13 15/15] vfio/type1: Return the MSI geometry through VFIO_IOMMU_GET_INFO capability chains
From: Auger Eric @ 2016-10-07 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006144251.46b45117@t450s.home>
Hi Alex,
On 06/10/2016 22:42, Alex Williamson wrote:
> On Thu, 6 Oct 2016 14:20:40 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
>> On Thu, 6 Oct 2016 08:45:31 +0000
>> Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> This patch allows the user-space to retrieve the MSI geometry. The
>>> implementation is based on capability chains, now also added to
>>> VFIO_IOMMU_GET_INFO.
>>>
>>> The returned info comprise:
>>> - whether the MSI IOVA are constrained to a reserved range (x86 case) and
>>> in the positive, the start/end of the aperture,
>>> - or whether the IOVA aperture need to be set by the userspace. In that
>>> case, the size and alignment of the IOVA window to be provided are
>>> returned.
>>>
>>> In case the userspace must provide the IOVA aperture, we currently report
>>> a size/alignment based on all the doorbells registered by the host kernel.
>>> This may exceed the actual needs.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>> v11 -> v11:
>>> - msi_doorbell_pages was renamed msi_doorbell_calc_pages
>>>
>>> v9 -> v10:
>>> - move cap_offset after iova_pgsizes
>>> - replace __u64 alignment by __u32 order
>>> - introduce __u32 flags in vfio_iommu_type1_info_cap_msi_geometry and
>>> fix alignment
>>> - call msi-doorbell API to compute the size/alignment
>>>
>>> v8 -> v9:
>>> - use iommu_msi_supported flag instead of programmable
>>> - replace IOMMU_INFO_REQUIRE_MSI_MAP flag by a more sophisticated
>>> capability chain, reporting the MSI geometry
>>>
>>> v7 -> v8:
>>> - use iommu_domain_msi_geometry
>>>
>>> v6 -> v7:
>>> - remove the computation of the number of IOVA pages to be provisionned.
>>> This number depends on the domain/group/device topology which can
>>> dynamically change. Let's rely instead rely on an arbitrary max depending
>>> on the system
>>>
>>> v4 -> v5:
>>> - move msi_info and ret declaration within the conditional code
>>>
>>> v3 -> v4:
>>> - replace former vfio_domains_require_msi_mapping by
>>> more complex computation of MSI mapping requirements, especially the
>>> number of pages to be provided by the user-space.
>>> - reword patch title
>>>
>>> RFC v1 -> v1:
>>> - derived from
>>> [RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs automap state
>>> - renamed allow_msi_reconfig into require_msi_mapping
>>> - fixed VFIO_IOMMU_GET_INFO
>>> ---
>>> drivers/vfio/vfio_iommu_type1.c | 78 ++++++++++++++++++++++++++++++++++++++++-
>>> include/uapi/linux/vfio.h | 32 ++++++++++++++++-
>>> 2 files changed, 108 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>>> index dc3ee5d..ce5e7eb 100644
>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>> @@ -38,6 +38,8 @@
>>> #include <linux/workqueue.h>
>>> #include <linux/dma-iommu.h>
>>> #include <linux/msi-doorbell.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/msi.h>
>>>
>>> #define DRIVER_VERSION "0.2"
>>> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
>>> @@ -1101,6 +1103,55 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
>>> return ret;
>>> }
>>>
>>> +static int compute_msi_geometry_caps(struct vfio_iommu *iommu,
>>> + struct vfio_info_cap *caps)
>>> +{
>>> + struct vfio_iommu_type1_info_cap_msi_geometry *vfio_msi_geometry;
>>> + unsigned long order = __ffs(vfio_pgsize_bitmap(iommu));
>>> + struct iommu_domain_msi_geometry msi_geometry;
>>> + struct vfio_info_cap_header *header;
>>> + struct vfio_domain *d;
>>> + bool reserved;
>>> + size_t size;
>>> +
>>> + mutex_lock(&iommu->lock);
>>> + /* All domains have same require_msi_map property, pick first */
>>> + d = list_first_entry(&iommu->domain_list, struct vfio_domain, next);
>>> + iommu_domain_get_attr(d->domain, DOMAIN_ATTR_MSI_GEOMETRY,
>>> + &msi_geometry);
>>> + reserved = !msi_geometry.iommu_msi_supported;
>>> +
>>> + mutex_unlock(&iommu->lock);
>>> +
>>> + size = sizeof(*vfio_msi_geometry);
>>> + header = vfio_info_cap_add(caps, size,
>>> + VFIO_IOMMU_TYPE1_INFO_CAP_MSI_GEOMETRY, 1);
>>> +
>>> + if (IS_ERR(header))
>>> + return PTR_ERR(header);
>>> +
>>> + vfio_msi_geometry = container_of(header,
>>> + struct vfio_iommu_type1_info_cap_msi_geometry,
>>> + header);
>>> +
>>> + vfio_msi_geometry->flags = reserved;
>>
>> Use the bit flag VFIO_IOMMU_MSI_GEOMETRY_RESERVED
>>
>>> + if (reserved) {
>>> + vfio_msi_geometry->aperture_start = msi_geometry.aperture_start;
>>> + vfio_msi_geometry->aperture_end = msi_geometry.aperture_end;
>>
>> But maybe nobody has set these, did you intend to use
>> iommu_domain_msi_aperture_valid(), which you defined early on but never
>> used?
>>
>>> + return 0;
>>> + }
>>> +
>>> + vfio_msi_geometry->order = order;
>>
>> I'm tempted to suggest that a user could do the same math on their own
>> since we provide the supported bitmap already... could it ever not be
>> the same?
>>
>>> + /*
>>> + * we compute a system-wide requirement based on all the registered
>>> + * doorbells
>>> + */
>>> + vfio_msi_geometry->size =
>>> + msi_doorbell_calc_pages(order) * ((uint64_t) 1 << order);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static long vfio_iommu_type1_ioctl(void *iommu_data,
>>> unsigned int cmd, unsigned long arg)
>>> {
>>> @@ -1122,8 +1173,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>>> }
>>> } else if (cmd == VFIO_IOMMU_GET_INFO) {
>>> struct vfio_iommu_type1_info info;
>>> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
>>> + int ret;
>>>
>>> - minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
>>> + minsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
>>>
>>> if (copy_from_user(&info, (void __user *)arg, minsz))
>>> return -EFAULT;
>>> @@ -1135,6 +1188,29 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>>>
>>> info.iova_pgsizes = vfio_pgsize_bitmap(iommu);
>>>
>>> + ret = compute_msi_geometry_caps(iommu, &caps);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (caps.size) {
>>> + info.flags |= VFIO_IOMMU_INFO_CAPS;
>>> + if (info.argsz < sizeof(info) + caps.size) {
>>> + info.argsz = sizeof(info) + caps.size;
>>> + info.cap_offset = 0;
>>> + } else {
>>> + vfio_info_cap_shift(&caps, sizeof(info));
>>> + if (copy_to_user((void __user *)arg +
>>> + sizeof(info), caps.buf,
>>> + caps.size)) {
>>> + kfree(caps.buf);
>>> + return -EFAULT;
>>> + }
>>> + info.cap_offset = sizeof(info);
>>> + }
>>> +
>>> + kfree(caps.buf);
>>> + }
>>> +
>>> return copy_to_user((void __user *)arg, &info, minsz) ?
>>> -EFAULT : 0;
>>>
>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>> index 4a9dbc2..8dae013 100644
>>> --- a/include/uapi/linux/vfio.h
>>> +++ b/include/uapi/linux/vfio.h
>>> @@ -488,7 +488,35 @@ struct vfio_iommu_type1_info {
>>> __u32 argsz;
>>> __u32 flags;
>>> #define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */
>>> - __u64 iova_pgsizes; /* Bitmap of supported page sizes */
>>> +#define VFIO_IOMMU_INFO_CAPS (1 << 1) /* Info supports caps */
>>> + __u64 iova_pgsizes; /* Bitmap of supported page sizes */
>>> + __u32 __resv;
>>> + __u32 cap_offset; /* Offset within info struct of first cap */
>>> +};
>>
>> I understand the padding, but not the ordering. Why not end with
>> padding?
>>
>>> +
>>> +#define VFIO_IOMMU_TYPE1_INFO_CAP_MSI_GEOMETRY 1
>>> +
>>> +/*
>>> + * The MSI geometry capability allows to report the MSI IOVA geometry:
>>> + * - either the MSI IOVAs are constrained within a reserved IOVA aperture
>>> + * whose boundaries are given by [@aperture_start, @aperture_end].
>>> + * this is typically the case on x86 host. The userspace is not allowed
>>> + * to map userspace memory at IOVAs intersecting this range using
>>> + * VFIO_IOMMU_MAP_DMA.
>>> + * - or the MSI IOVAs are not requested to belong to any reserved range;
>>> + * in that case the userspace must provide an IOVA window characterized by
>>> + * @size and @alignment using VFIO_IOMMU_MAP_DMA with RESERVED_MSI_IOVA flag.
>>> + */
>>> +struct vfio_iommu_type1_info_cap_msi_geometry {
>>> + struct vfio_info_cap_header header;
>>> + __u32 flags;
>>> +#define VFIO_IOMMU_MSI_GEOMETRY_RESERVED (1 << 0) /* reserved geometry */
>>> + /* not reserved */
>>> + __u32 order; /* iommu page order used for aperture alignment*/
>>> + __u64 size; /* IOVA aperture size (bytes) the userspace must provide */
>>> + /* reserved */
>>> + __u64 aperture_start;
>>> + __u64 aperture_end;
>>
>> Should these be a union? We never set them both. Should the !reserved
>> case have a flag as well, so the user can positively identify what's
>> being provided?
>
> Actually, is there really any need to fit both of these within the same
> structure? Part of the idea of the capability chains is we can create
> a capability for each new thing we want to describe. So, we could
> simply define a generic reserved IOVA range capability with a 'start'
> and 'end' and then another capability to define MSI mapping
> requirements. Thanks,
Yes your suggested approach makes sense to me.
One reason why I proceeded that way is we are mixing things at iommu.h
level too. Personally I would have preferred to separate things:
1) add a new IOMMU_CAP_TRANSLATE_MSI capability in iommu_cap
2) rename iommu_msi_supported into "programmable" bool: reporting
whether the aperture is reserved or programmable.
In the early releases I think it was as above but slightly we moved to a
mixed description.
What do you think?
Thank you for the whole review!
Eric
>
> Alex
>
>>> };
>>>
>>> #define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>> @@ -503,6 +531,8 @@ struct vfio_iommu_type1_info {
>>> * IOVA region that will be used on some platforms to map the host MSI frames.
>>> * In that specific case, vaddr is ignored. Once registered, an MSI reserved
>>> * IOVA region stays until the container is closed.
>>> + * The requirement for provisioning such reserved IOVA range can be checked by
>>> + * checking the VFIO_IOMMU_TYPE1_INFO_CAP_MSI_GEOMETRY capability.
>>> */
>>> struct vfio_iommu_type1_dma_map {
>>> __u32 argsz;
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v13 12/15] vfio: Allow reserved msi iova registration
From: Auger Eric @ 2016-10-07 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006141932.0a193f9b@t450s.home>
Hi Alex,
On 06/10/2016 22:19, Alex Williamson wrote:
> On Thu, 6 Oct 2016 08:45:28 +0000
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> The user is allowed to register a reserved MSI IOVA range by using the
>> DMA MAP API and setting the new flag: VFIO_DMA_MAP_FLAG_MSI_RESERVED_IOVA.
>> This region is stored in the vfio_dma rb tree. At that point the iova
>> range is not mapped to any target address yet. The host kernel will use
>> those iova when needed, typically when MSIs are allocated.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
>>
>> ---
>> v12 -> v13:
>> - use iommu_get_dma_msi_region_cookie
>>
>> v9 -> v10
>> - use VFIO_IOVA_RESERVED_MSI enum value
>>
>> v7 -> v8:
>> - use iommu_msi_set_aperture function. There is no notion of
>> unregistration anymore since the reserved msi slot remains
>> until the container gets closed.
>>
>> v6 -> v7:
>> - use iommu_free_reserved_iova_domain
>> - convey prot attributes downto dma-reserved-iommu iova domain creation
>> - reserved bindings teardown now performed on iommu domain destruction
>> - rename VFIO_DMA_MAP_FLAG_MSI_RESERVED_IOVA into
>> VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA
>> - change title
>> - pass the protection attribute to dma-reserved-iommu API
>>
>> v3 -> v4:
>> - use iommu_alloc/free_reserved_iova_domain exported by dma-reserved-iommu
>> - protect vfio_register_reserved_iova_range implementation with
>> CONFIG_IOMMU_DMA_RESERVED
>> - handle unregistration by user-space and on vfio_iommu_type1 release
>>
>> v1 -> v2:
>> - set returned value according to alloc_reserved_iova_domain result
>> - free the iova domains in case any error occurs
>>
>> RFC v1 -> v1:
>> - takes into account Alex comments, based on
>> [RFC PATCH 1/6] vfio: Add interface for add/del reserved iova region:
>> - use the existing dma map/unmap ioctl interface with a flag to register
>> a reserved IOVA range. A single reserved iova region is allowed.
>> ---
>> drivers/vfio/vfio_iommu_type1.c | 77 ++++++++++++++++++++++++++++++++++++++++-
>> include/uapi/linux/vfio.h | 10 +++++-
>> 2 files changed, 85 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>> index 5bc5fc9..c2f8bd9 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -442,6 +442,20 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
>> vfio_lock_acct(-unlocked);
>> }
>>
>> +static int vfio_set_msi_aperture(struct vfio_iommu *iommu,
>> + dma_addr_t iova, size_t size)
>> +{
>> + struct vfio_domain *d;
>> + int ret = 0;
>> +
>> + list_for_each_entry(d, &iommu->domain_list, next) {
>> + ret = iommu_get_dma_msi_region_cookie(d->domain, iova, size);
>> + if (ret)
>> + break;
>> + }
>> + return ret;
>
> Doesn't this need an unwind on failure loop?
At the moment the de-allocation is done by the smmu driver, on
domain_free ops, which calls iommu_put_dma_cookie. In case,
iommu_get_dma_msi_region_cookie fails on a given VFIO domain currently
there is no other way but destroying all VFIO domains and redo everything.
So yes I plan to unfold everything, ie call iommu_put_dma_cookie for
each domain.
>
>> +}
>> +
>> static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
>> {
>> vfio_unmap_unpin(iommu, dma);
>> @@ -691,6 +705,63 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>> return ret;
>> }
>>
>> +static int vfio_register_msi_range(struct vfio_iommu *iommu,
>> + struct vfio_iommu_type1_dma_map *map)
>> +{
>> + dma_addr_t iova = map->iova;
>> + size_t size = map->size;
>> + int ret = 0;
>> + struct vfio_dma *dma;
>> + unsigned long order;
>> + uint64_t mask;
>> +
>> + /* Verify that none of our __u64 fields overflow */
>> + if (map->size != size || map->iova != iova)
>> + return -EINVAL;
>> +
>> + order = __ffs(vfio_pgsize_bitmap(iommu));
>> + mask = ((uint64_t)1 << order) - 1;
>> +
>> + WARN_ON(mask & PAGE_MASK);
>> +
>> + if (!size || (size | iova) & mask)
>> + return -EINVAL;
>> +
>> + /* Don't allow IOVA address wrap */
>> + if (iova + size - 1 < iova)
>> + return -EINVAL;
>> +
>> + mutex_lock(&iommu->lock);
>> +
>> + if (vfio_find_dma(iommu, iova, size, VFIO_IOVA_ANY)) {
>> + ret = -EEXIST;
>> + goto unlock;
>> + }
>> +
>> + dma = kzalloc(sizeof(*dma), GFP_KERNEL);
>> + if (!dma) {
>> + ret = -ENOMEM;
>> + goto unlock;
>> + }
>> +
>> + dma->iova = iova;
>> + dma->size = size;
>> + dma->type = VFIO_IOVA_RESERVED_MSI;
>> +
>> + ret = vfio_set_msi_aperture(iommu, iova, size);
>> + if (ret)
>> + goto free_unlock;
>> +
>> + vfio_link_dma(iommu, dma);
>> + goto unlock;
>> +
>> +free_unlock:
>> + kfree(dma);
>> +unlock:
>> + mutex_unlock(&iommu->lock);
>> + return ret;
>> +}
>> +
>> static int vfio_bus_type(struct device *dev, void *data)
>> {
>> struct bus_type **bus = data;
>> @@ -1064,7 +1135,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>> } else if (cmd == VFIO_IOMMU_MAP_DMA) {
>> struct vfio_iommu_type1_dma_map map;
>> uint32_t mask = VFIO_DMA_MAP_FLAG_READ |
>> - VFIO_DMA_MAP_FLAG_WRITE;
>> + VFIO_DMA_MAP_FLAG_WRITE |
>> + VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA;
>>
>> minsz = offsetofend(struct vfio_iommu_type1_dma_map, size);
>>
>> @@ -1074,6 +1146,9 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>> if (map.argsz < minsz || map.flags & ~mask)
>> return -EINVAL;
>>
>> + if (map.flags & VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA)
>> + return vfio_register_msi_range(iommu, &map);
>> +
>> return vfio_dma_do_map(iommu, &map);
>>
>> } else if (cmd == VFIO_IOMMU_UNMAP_DMA) {
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index 255a211..4a9dbc2 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -498,12 +498,19 @@ struct vfio_iommu_type1_info {
>> *
>> * Map process virtual addresses to IO virtual addresses using the
>> * provided struct vfio_dma_map. Caller sets argsz. READ &/ WRITE required.
>> + *
>> + * In case RESERVED_MSI_IOVA flag is set, the API only aims at registering an
>> + * IOVA region that will be used on some platforms to map the host MSI frames.
>> + * In that specific case, vaddr is ignored. Once registered, an MSI reserved
>> + * IOVA region stays until the container is closed.
>> */
>> struct vfio_iommu_type1_dma_map {
>> __u32 argsz;
>> __u32 flags;
>> #define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device */
>> #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */
>> +/* reserved iova for MSI vectors*/
>> +#define VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA (1 << 2)
>> __u64 vaddr; /* Process virtual address */
>> __u64 iova; /* IO virtual address */
>> __u64 size; /* Size of mapping (bytes) */
>> @@ -519,7 +526,8 @@ struct vfio_iommu_type1_dma_map {
>> * Caller sets argsz. The actual unmapped size is returned in the size
>> * field. No guarantee is made to the user that arbitrary unmaps of iova
>> * or size different from those used in the original mapping call will
>> - * succeed.
>> + * succeed. Once registered, an MSI region cannot be unmapped and stays
>> + * until the container is closed.
>> */
>> struct vfio_iommu_type1_dma_unmap {
>> __u32 argsz;
>
> What happens when an x86 user does a mapping with this new flag set?
> It seems like we end up configuring everything just as we would on a
> platform requiring MSI mapping, including setting the domain MSI
> geometry. Should we be testing the MSI geometry flag on the iommu to
> see if this is supported? Surprisingly few things seem to check that
> flag.
Yes I need to test the capability first and return -EINVAL in case the
capability is not supported..
Thanks
Eric
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH/RFT 11/12] ARM: dts: da850: Add the usb ohci device node
From: Sergei Shtylyov @ 2016-10-07 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475858577-10366-12-git-send-email-ahaslam@baylibre.com>
Hello.
On 10/07/2016 07:42 PM, ahaslam at baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> This adds the device tree node for the usb11 (ohci)
> controller present in the da850 family of SoC's.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 33fcdce..afae565 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -381,6 +381,14 @@
> #phy-cells = <1>;
> status = "disabled";
> };
> + usb11: usb11 at 0225000 {
No, usb@<unit-address> to comply with the DT spec the node names should be
generic.
[...]
MBR, Sergei
^ permalink raw reply
* [PATCH v13 11/15] vfio/type1: Handle unmap/unpin and replay for VFIO_IOVA_RESERVED slots
From: Auger Eric @ 2016-10-07 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006141918.67928391@t450s.home>
Hi Alex,
On 06/10/2016 22:19, Alex Williamson wrote:
> On Thu, 6 Oct 2016 08:45:27 +0000
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Before allowing the end-user to create VFIO_IOVA_RESERVED dma slots,
>> let's implement the expected behavior for removal and replay.
>>
>> As opposed to user dma slots, reserved IOVAs are not systematically bound
>> to PAs and PAs are not pinned. VFIO just initializes the IOVA "aperture".
>> IOVAs are allocated outside of the VFIO framework, by the MSI layer which
>> is responsible to free and unmap them. The MSI mapping resources are freeed
>
> nit, extra 'e', "freed"
>
>> by the IOMMU driver on domain destruction.
>>
>> On the creation of a new domain, the "replay" of a reserved slot simply
>> needs to set the MSI aperture on the new domain.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v12 -> v13:
>> - use dma-iommu iommu_get_dma_msi_region_cookie
>>
>> v9 -> v10:
>> - replay of a reserved slot sets the MSI aperture on the new domain
>> - use VFIO_IOVA_RESERVED_MSI enum value instead of VFIO_IOVA_RESERVED
>>
>> v7 -> v8:
>> - do no destroy anything anymore, just bypass unmap/unpin and iommu_map
>> on replay
>> ---
>> drivers/vfio/Kconfig | 1 +
>> drivers/vfio/vfio_iommu_type1.c | 10 +++++++++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
>> index da6e2ce..673ec79 100644
>> --- a/drivers/vfio/Kconfig
>> +++ b/drivers/vfio/Kconfig
>> @@ -1,6 +1,7 @@
>> config VFIO_IOMMU_TYPE1
>> tristate
>> depends on VFIO
>> + select IOMMU_DMA
>> default n
>>
>> config VFIO_IOMMU_SPAPR_TCE
>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>> index 65a4038..5bc5fc9 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -36,6 +36,7 @@
>> #include <linux/uaccess.h>
>> #include <linux/vfio.h>
>> #include <linux/workqueue.h>
>> +#include <linux/dma-iommu.h>
>>
>> #define DRIVER_VERSION "0.2"
>> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
>> @@ -387,7 +388,7 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
>> struct vfio_domain *domain, *d;
>> long unlocked = 0;
>>
>> - if (!dma->size)
>> + if (!dma->size || dma->type != VFIO_IOVA_USER)
>> return;
>> /*
>> * We use the IOMMU to track the physical addresses, otherwise we'd
>> @@ -724,6 +725,13 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
>> dma = rb_entry(n, struct vfio_dma, node);
>> iova = dma->iova;
>>
>> + if (dma->type == VFIO_IOVA_RESERVED_MSI) {
>> + ret = iommu_get_dma_msi_region_cookie(domain->domain,
>> + dma->iova, dma->size);
>> + WARN_ON(ret);
>> + continue;
>> + }
>
> Why is this a passable error? We consider an iommu_map() error on any
> entry a failure.
Yes I agree.
Thanks
Eric
>
>> +
>> while (iova < dma->iova + dma->size) {
>> phys_addr_t phys = iommu_iova_to_phys(d->domain, iova);
>> size_t size;
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API
From: Auger Eric @ 2016-10-07 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006141750.1e209e26@t450s.home>
Hi Alex,
On 06/10/2016 22:17, Alex Williamson wrote:
> On Thu, 6 Oct 2016 08:45:20 +0000
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> We introduce a new msi-doorbell API that allows msi controllers
>> to allocate and register their doorbells. This is useful when
>> those doorbells are likely to be iommu mapped (typically on ARM).
>> The VFIO layer will need to gather information about those doorbells:
>> whether they are safe (ie. they implement irq remapping) and how
>> many IOMMU pages are requested to map all of them.
>>
>> This patch first introduces the dedicated msi_doorbell_info struct
>> and the registration/unregistration functions.
>>
>> A doorbell region is characterized by its physical address base, size,
>> and whether it its safe (ie. it implements IRQ remapping). A doorbell
>> can be per-cpu of global. We currently only care about global doorbells.
> ^^ s/of/or/
OK
>
>>
>> A function returns whether all doorbells are safe.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v12 -> v13:
>> - directly select MSI_DOORBELL in ARM_SMMU and ARM_SMMU_V3 configs
>> - remove prot attribute
>> - move msi_doorbell_info struct definition in msi-doorbell.c
>> - change the commit title
>> - change proto of the registration function
>> - msi_doorbell_safe now in this patch
>>
>> v11 -> v12:
>> - rename irqchip_doorbell into msi_doorbell, irqchip_doorbell_list
>> into msi_doorbell_list and irqchip_doorbell_mutex into
>> msi_doorbell_mutex
>> - fix style issues: align msi_doorbell struct members, kernel-doc comments
>> - use kzalloc
>> - use container_of in msi_doorbell_unregister_global
>> - compute nb_unsafe_doorbells on registration/unregistration
>> - registration simply returns NULL if allocation failed
>>
>> v10 -> v11:
>> - remove void *chip_data argument from register/unregister function
>> - remove lookup funtions since we restored the struct irq_chip
>> msi_doorbell_info ops to realize this function
>> - reword commit message and title
>>
>> Conflicts:
>> kernel/irq/Makefile
>>
>> Conflicts:
>> drivers/iommu/Kconfig
>> ---
>> drivers/iommu/Kconfig | 2 +
>> include/linux/msi-doorbell.h | 77 ++++++++++++++++++++++++++++++++++
>> kernel/irq/Kconfig | 4 ++
>> kernel/irq/Makefile | 1 +
>> kernel/irq/msi-doorbell.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 182 insertions(+)
>> create mode 100644 include/linux/msi-doorbell.h
>> create mode 100644 kernel/irq/msi-doorbell.c
>>
>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>> index 8ee54d7..0cc7fac 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -297,6 +297,7 @@ config SPAPR_TCE_IOMMU
>> config ARM_SMMU
>> bool "ARM Ltd. System MMU (SMMU) Support"
>> depends on (ARM64 || ARM) && MMU
>> + select MSI_DOORBELL
>> select IOMMU_API
>> select IOMMU_IO_PGTABLE_LPAE
>> select ARM_DMA_USE_IOMMU if ARM
>> @@ -310,6 +311,7 @@ config ARM_SMMU
>> config ARM_SMMU_V3
>> bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
>> depends on ARM64
>> + select MSI_DOORBELL
>> select IOMMU_API
>> select IOMMU_IO_PGTABLE_LPAE
>> select GENERIC_MSI_IRQ_DOMAIN
>> diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
>> new file mode 100644
>> index 0000000..c18a382
>> --- /dev/null
>> +++ b/include/linux/msi-doorbell.h
>> @@ -0,0 +1,77 @@
>> +/*
>> + * API to register/query MSI doorbells likely to be IOMMU mapped
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef _LINUX_MSI_DOORBELL_H
>> +#define _LINUX_MSI_DOORBELL_H
>> +
>> +struct msi_doorbell_info;
>> +
>> +#ifdef CONFIG_MSI_DOORBELL
>> +
>> +/**
>> + * msi_doorbell_register - allocate and register a global doorbell
>> + * @base: physical base address of the global doorbell
>> + * @size: size of the global doorbell
>> + * @prot: protection/memory attributes
>> + * @safe: true is irq_remapping implemented for this doorbell
>> + * @dbinfo: returned doorbell info
>> + *
>> + * Return: 0 on success, -ENOMEM on allocation failure
>> + */
>> +int msi_doorbell_register_global(phys_addr_t base, size_t size,
>> + bool safe,
>> + struct msi_doorbell_info **dbinfo);
>> +
>
> Seems like alloc/free behavior vs register/unregister. Also seems
> cleaner to just return a struct msi_doorbell_info* and use PTR_ERR for
> return codes. These are of course superficial changes that could be
> addressed in the future.
Sure
>
>> +/**
>> + * msi_doorbell_unregister_global - unregister a global doorbell
>> + * @db: doorbell info to unregister
>> + *
>> + * remove the doorbell descriptor from the list of registered doorbells
>> + * and deallocates it
>> + */
>> +void msi_doorbell_unregister_global(struct msi_doorbell_info *db);
>> +
>> +/**
>> + * msi_doorbell_safe - return whether all registered doorbells are safe
>> + *
>> + * Safe doorbells are those which implement irq remapping
>> + * Return: true if all doorbells are safe, false otherwise
>> + */
>> +bool msi_doorbell_safe(void);
>> +
>> +#else
>> +
>> +static inline int
>> +msi_doorbell_register_global(phys_addr_t base, size_t size,
>> + int prot, bool safe,
>> + struct msi_doorbell_info **dbinfo)
>> +{
>> + *dbinfo = NULL;
>> + return 0;
>
> If we return a struct*
>
> return NULL;
Yep
>
>> +}
>> +
>> +static inline void
>> +msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
>> +
>> +static inline bool msi_doorbell_safe(void)
>> +{
>> + return true;
>> +}
>
> Is it?
Yes I will return false and change the safety check in vfio_iommu_type1.c
Thanks
Eric
>
>> +#endif /* CONFIG_MSI_DOORBELL */
>> +
>> +#endif
>> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
>> index 3bbfd6a..d4faaaa 100644
>> --- a/kernel/irq/Kconfig
>> +++ b/kernel/irq/Kconfig
>> @@ -72,6 +72,10 @@ config GENERIC_IRQ_IPI
>> config GENERIC_MSI_IRQ
>> bool
>>
>> +# MSI doorbell support (for doorbell IOMMU mapping)
>> +config MSI_DOORBELL
>> + bool
>> +
>> # Generic MSI hierarchical interrupt domain support
>> config GENERIC_MSI_IRQ_DOMAIN
>> bool
>> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
>> index 1d3ee31..5b04dd1 100644
>> --- a/kernel/irq/Makefile
>> +++ b/kernel/irq/Makefile
>> @@ -10,3 +10,4 @@ obj-$(CONFIG_PM_SLEEP) += pm.o
>> obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
>> obj-$(CONFIG_GENERIC_IRQ_IPI) += ipi.o
>> obj-$(CONFIG_SMP) += affinity.o
>> +obj-$(CONFIG_MSI_DOORBELL) += msi-doorbell.o
>> diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
>> new file mode 100644
>> index 0000000..60a262a
>> --- /dev/null
>> +++ b/kernel/irq/msi-doorbell.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * API to register/query MSI doorbells likely to be IOMMU mapped
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc.
>> + * Author: Eric Auger <eric.auger@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/slab.h>
>> +#include <linux/irq.h>
>> +#include <linux/msi-doorbell.h>
>> +
>> +/**
>> + * struct msi_doorbell_info - MSI doorbell region descriptor
>> + * @percpu_doorbells: per cpu doorbell base address
>> + * @global_doorbell: base address of the doorbell
>> + * @doorbell_is_percpu: is the doorbell per cpu or global?
>> + * @safe: true if irq remapping is implemented
>> + * @size: size of the doorbell
>> + */
>> +struct msi_doorbell_info {
>> + union {
>> + phys_addr_t __percpu *percpu_doorbells;
>> + phys_addr_t global_doorbell;
>> + };
>> + bool doorbell_is_percpu;
>> + bool safe;
>> + size_t size;
>> +};
>> +
>> +struct msi_doorbell {
>> + struct msi_doorbell_info info;
>> + struct list_head next;
>> +};
>> +
>> +/* list of registered MSI doorbells */
>> +static LIST_HEAD(msi_doorbell_list);
>> +
>> +/* counts the number of unsafe registered doorbells */
>> +static uint nb_unsafe_doorbells;
>> +
>> +/* protects the list and nb__unsafe_doorbells */
>
> Extra underscore
>
>> +static DEFINE_MUTEX(msi_doorbell_mutex);
>> +
>> +int msi_doorbell_register_global(phys_addr_t base, size_t size, bool safe,
>> + struct msi_doorbell_info **dbinfo)
>> +{
>> + struct msi_doorbell *db;
>> +
>> + db = kzalloc(sizeof(*db), GFP_KERNEL);
>> + if (!db)
>> + return -ENOMEM;
>> +
>> + db->info.global_doorbell = base;
>> + db->info.size = size;
>> + db->info.safe = safe;
>> +
>> + mutex_lock(&msi_doorbell_mutex);
>> + list_add(&db->next, &msi_doorbell_list);
>> + if (!db->info.safe)
>> + nb_unsafe_doorbells++;
>> + mutex_unlock(&msi_doorbell_mutex);
>> + *dbinfo = &db->info;
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_register_global);
>> +
>> +void msi_doorbell_unregister_global(struct msi_doorbell_info *dbinfo)
>> +{
>> + struct msi_doorbell *db;
>> +
>> + db = container_of(dbinfo, struct msi_doorbell, info);
>> +
>> + mutex_lock(&msi_doorbell_mutex);
>> + list_del(&db->next);
>> + if (!db->info.safe)
>> + nb_unsafe_doorbells--;
>> + mutex_unlock(&msi_doorbell_mutex);
>> + kfree(db);
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
>> +
>> +bool msi_doorbell_safe(void)
>> +{
>> + return !nb_unsafe_doorbells;
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_safe);
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v13 03/15] iommu/dma: Allow MSI-only cookies
From: Auger Eric @ 2016-10-07 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161006141717.13c9c111@t450s.home>
Hi Alex,
On 06/10/2016 22:17, Alex Williamson wrote:
> On Thu, 6 Oct 2016 08:45:19 +0000
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> IOMMU domain users such as VFIO face a similar problem to DMA API ops
>> with regard to mapping MSI messages in systems where the MSI write is
>> subject to IOMMU translation. With the relevant infrastructure now in
>> place for managed DMA domains, it's actually really simple for other
>> users to piggyback off that and reap the benefits without giving up
>> their own IOVA management, and without having to reinvent their own
>> wheel in the MSI layer.
>>
>> Allow such users to opt into automatic MSI remapping by dedicating a
>> region of their IOVA space to a managed cookie.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - compared to Robin's version
>> - add NULL last param to iommu_dma_init_domain
>> - set the msi_geometry aperture
>> - I removed
>> if (base < U64_MAX - size)
>> reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
>> don't get why we would reserve something out of the scope of the iova domain?
>> what do I miss?
>> ---
>> drivers/iommu/dma-iommu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> include/linux/dma-iommu.h | 9 +++++++++
>> 2 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index c5ab866..11da1a0 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -716,3 +716,43 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>> msg->address_lo += lower_32_bits(msi_page->iova);
>> }
>> }
>> +
>> +/**
>> + * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
>
> Should this perhaps be iommu_setup_dma_msi_region_cookie, or something
> along those lines. I'm not sure what we're get'ing. Thanks,
This was chosen by analogy with legacy iommu_get_dma_cookie/
iommu_put_dma_cookie. But in practice it does both get &
iommu_dma_init_domain.
I plan to rename into iommu_setup_dma_msi_region if no objection
Thanks
Eric
>
> Alex
>
>> + * @domain: IOMMU domain to prepare
>> + * @base: Base address of IOVA region to use as the MSI remapping aperture
>> + * @size: Size of the desired MSI aperture
>> + *
>> + * Users who manage their own IOVA allocation and do not want DMA API support,
>> + * but would still like to take advantage of automatic MSI remapping, can use
>> + * this to initialise their own domain appropriately.
>> + */
>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>> + dma_addr_t base, u64 size)
>> +{
>> + struct iommu_dma_cookie *cookie;
>> + struct iova_domain *iovad;
>> + int ret;
>> +
>> + if (domain->type == IOMMU_DOMAIN_DMA)
>> + return -EINVAL;
>> +
>> + ret = iommu_get_dma_cookie(domain);
>> + if (ret)
>> + return ret;
>> +
>> + ret = iommu_dma_init_domain(domain, base, size, NULL);
>> + if (ret) {
>> + iommu_put_dma_cookie(domain);
>> + return ret;
>> + }
>> +
>> + domain->msi_geometry.aperture_start = base;
>> + domain->msi_geometry.aperture_end = base + size - 1;
>> +
>> + cookie = domain->iova_cookie;
>> + iovad = &cookie->iovad;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
>> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
>> index 32c5890..1c55413 100644
>> --- a/include/linux/dma-iommu.h
>> +++ b/include/linux/dma-iommu.h
>> @@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
>> /* The DMA API isn't _quite_ the whole story, though... */
>> void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
>>
>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>> + dma_addr_t base, u64 size);
>> +
>> #else
>>
>> struct iommu_domain;
>> @@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>> {
>> }
>>
>> +static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>> + dma_addr_t base, u64 size)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> #endif /* CONFIG_IOMMU_DMA */
>> #endif /* __KERNEL__ */
>> #endif /* __DMA_IOMMU_H */
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pavel Labath @ 2016-10-07 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHB_GupyUp_s4nWq-QVnEoCZvmkrXyCcxs-XpH3x0Yugkknpzg@mail.gmail.com>
On 7 October 2016 at 09:38, Pratyush Anand <panand@redhat.com> wrote:
>
>
> IIUC, then you see an issue when an address watched is not the base
> address accessed by the instruction. For example, if an address 'a+8'
> is watched and an instruction accesses instruction from a to a +16. I
> tried to reproduce the issue with mustang using your test-case in
> patch3 (after couple of syntax modifcations for resolving compilation
> issue with gcc). All the test case did pass with existing code in
> v4.8. I noticed that, watchpoint exception is generated if any of the
> sub-location accessed from a single instruction is watched, provided
> watchdpoint watches either a byte, half word, word or double word
> from the base.
>
>
> So, either I must be missing something or the problem is not related
> to all arm64 platform.
Hello Pratyush,
Thank you for looking into this.
The thing is, I have observed different behavior here depending on the
exact hardware used. I don't have the exact parameters with me now,
but I can look it up next week.
The thing is that the spec is imprecise about what exact address the
hardware can report for the watchpoint hit. I presume that is
deliberate to give some leeway to implementers. The spec says the
address can be anywhere in the range from the lowest memory address
accessed by the instruction to the highest address watched by the
watchpoint, but most hardware seems to be stricter than that and
return an address that fits inside the watched range.
On chip 1, I observed the behavior where the hardware would
consistently report an address out of range of the watchpoint and we
would just spin it in a loop.
On chip 2, I observed the behavior where the hardware would report an
out-of-range address for the first two dozen (~) iterations, after
which it would "give up" and report an address that we were happy
with. I don't really have an explanation for this - I can only assume
that some external event like a reschedule to a different core caused
some internal state of the hardware to be reset and cause it to report
a different (better?) address instead. In the case where this was
happening, it had no observable effects on userspace - it did not see
the fact that we had re-executed the offending instruction a dozen
times and as far as it was concerned, the watchpoint functionality
worked perfectly. You can check whether this is happening in your case
by instrumenting the code to print the reported address whenever it
enters `watchpoint_handler`.
(I am sorry about the test errors. I was compiling the test case with
an android gcc - I'll make sure to check it with a vanilla linux gcc
also.)
>
> However, I did notice that it does not work if we watch an address
> which is at some offset from address programmed. For example, it works
> when byte_mask is 0x3, but it does not work if byte_mask if 0x2 (which
> is supported by hardware).
>
> I do have some patches to resolve that.
>
> https://github.com/pratyushanand/linux/commits/perf/upstream_arm64_devel
>
> I will send them for review comment after some testing.
I am looking forward to these patches - they were the next on my list
to look into after I got this resolved. :)
However: Are sure about 0x2 not being a valid byte mask? According to
my reading of the armv8 spec (section D7.3.11, "DBGWCR<n>_EL1, Debug
Watchpoint Control Registers, n = 0 - 15") it should be fine.
====
The valid values for BAS are 0b0000000, or a binary number all of
whose set bits are contiguous. All other values are reserved and must
not be used by software.
====
So, 0x2 (as well as 0x6, 0xC, 0xE) should be fine as it has a
contiguous sequence of set bit(s). I haven't tried yet whether any
hardware actually handles that correctly, but I was certainly hoping
we would be able to watch more precise memory regions.
regards,
Pavel
^ permalink raw reply
* [PATCH 1/4] staging/vchi: Convert to current get_user_pages() arguments.
From: Eric Anholt @ 2016-10-07 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007144412.GA27514@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Mon, Oct 03, 2016 at 11:52:06AM -0700, Eric Anholt wrote:
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>
> In the future, it's good to put at least some changelog text in here. I
> normally do not accept patches without it. But I'll take these two.
I'm generally verbose in commit messages, but these seemed like they
didn't have useful context to be added.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161007/5a2699fc/attachment.sig>
^ permalink raw reply
* [PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
From: Peter Griffin @ 2016-10-07 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACvgo50vU5_=KFUCAZDaQPepPdsbnF8KN1b+f6KJNuBW6qRdSg@mail.gmail.com>
Hi Emil,
On Thu, 06 Oct 2016, Emil Velikov wrote:
> On 6 October 2016 at 10:37, Peter Griffin <peter.griffin@linaro.org> wrote:
>
> > In fact the help text for VIRTIO even states this option should be selected
> > by any driver which implements virtio.
> >
> Almost but not quite. It says:
>
> "This option is selected by any driver which implements the virtio _bus_"
>
Ah I thought DRM_VIRTIO_GPU was implementing a virtio bus, bus it seems that it
uses pci. Which does raise the question of why it is depending on VIRTIO at all
and not VIRTIO_PCI.
> REMOTEPROC obviously does that while the ST SLIM driver does not. Thus
> the latter should _not_ select, be that explicitly or implicitly via
> REMOTEPROC, the symbol.
Yep OK.
>
> >>
> >> People tend to abuse select because it's "convenient". If you depend,
> >> but some of your dependencies aren't met, you're in for some digging
> >> through Kconfig to find the missing deps. Just to make the option you
> >> want visible in menuconfig. If you instead select something with
> >> dependencies, it'll be right most of the time, and it's "convenient",
> >> until it breaks. (And hey, it usually breaks for someone else with some
> >> other config, so it's still convenient for you.)
> >
> > I'm sure they do but in this case it is actually the use of 'depends on'
> > which has caused the breakage and inconvenience for somebody else and sadly this
> > inconvienice is still on-going due to this patch not being applied or getting an
> > acked-by from the appropriate maintainers.
> >
> Surely you're not saying that pre-existing driver following the
> documentation, is 'causing breakage' for a new driver {ab,mis}using a
> feature ?
Your right I wasn't saying that :)
My point was that this patch wasn't 'wrong' when referring to the Kconfig
documentation Jani referenced as VIRTIO has no dependencies.
Also I thought DRM_VIRTIO_GPU driver implemented a VIRTIO bus which re-enforced
the view that it should be selecting VIRTIO.
>
> This reminds me an old saying: "If the shoe doesn?t fit, it doesn?t
> mean there is anything wrong with your feet."
If the shoe doesn't fit, chop off the leg :)
> You seem to be suggesting the opposite ?
>
> >>
> >> Perhaps kconfig should complain about selecting visible symbols and
> >> symbols with dependencies.
> >
> > That sounds like it would be a useful addition.
> >
> > Is it possible to get this patch applied or an acked-by to avoid further delay
> > to the fdma series?
> >
> Please don't apply duct tape, especially where it's _not_ needed.
>
> $ sed -i s/select REMOTEPROC/depends on REMOTEPROC/ drivers/remoteproc/Kconfig
>
> ... will resolve things in the right place. The alternative will lead
> to random issues in other subsystems.
>
If Bjorn is OK with it, then it is fine with me.
I will update remoteproc Kconfig setup in fdma v10, this will drop the
requirement for this patch in drm subsystem.
I can then send the whitespace cleanup patch separately to DRM ML.
regards,
Peter.
^ permalink raw reply
* [PATCH] tty: serial: fsl_lpuart: Fix Tx DMA edge case
From: Stefan Agner @ 2016-10-07 18:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475791984-17705-1-git-send-email-aaron.brice@datasoft.com>
On 2016-10-06 15:13, Aaron Brice wrote:
> In the case where head == 0 on the circular buffer, there should be one
> DMA buffer, not two. The second zero-length buffer would break the
> lpuart driver, transfer would never complete.
That looks right, and seems to work fine here:
Acked-by: Stefan Agner <stefan@agner.ch>
Tested-by: Stefan Agner <stefan@agner.ch>
@Greg, would be good if this would still make it into 4.9.
--
Stefan
>
> Signed-off-by: Aaron Brice <aaron.brice@datasoft.com>
> ---
> drivers/tty/serial/fsl_lpuart.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index de9d510..76103f2 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -328,7 +328,7 @@ static void lpuart_dma_tx(struct lpuart_port *sport)
>
> sport->dma_tx_bytes = uart_circ_chars_pending(xmit);
>
> - if (xmit->tail < xmit->head) {
> + if (xmit->tail < xmit->head || xmit->head == 0) {
> sport->dma_tx_nents = 1;
> sg_init_one(sgl, xmit->buf + xmit->tail, sport->dma_tx_bytes);
> } else {
> @@ -359,7 +359,6 @@ static void lpuart_dma_tx(struct lpuart_port *sport)
> sport->dma_tx_in_progress = true;
> sport->dma_tx_cookie = dmaengine_submit(sport->dma_tx_desc);
> dma_async_issue_pending(sport->dma_tx_chan);
> -
> }
>
> static void lpuart_dma_tx_complete(void *arg)
^ permalink raw reply
* [PATCH 1/3] fpga manager: Add cyclonespi driver for Altera fpgas
From: atull @ 2016-10-07 18:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAtXAHcWLpn3DFP6YLWqBxsMQ0O=HNNCmB+kHbHhyAHFHQUbwA@mail.gmail.com>
On Fri, 7 Oct 2016, Moritz Fischer wrote:
> > +static inline u32 revbit8x4(u32 n)
> > +{
> > + n = ((n & 0xF0F0F0F0UL) >> 4) | ((n & 0x0F0F0F0FUL) << 4);
> > + n = ((n & 0xCCCCCCCCUL) >> 2) | ((n & 0x33333333UL) << 2);
> > + n = ((n & 0xAAAAAAAAUL) >> 1) | ((n & 0x55555555UL) << 1);
> > + return n;
> > +}
>
> During the Zynq FPGA manager reviews we decided that manipulating the bitstream
> to be consumable by the driver is userland's job.
Moritz, Can you remind me what that issue was there (or point me to
that email, I can't find it)? I don't think I had a problem with that
in your case. In general I think if these drivers can take the
bitstream that comes from the manufacturer's tools and stuff it into
the FPGA, then we are accomplishing what we want. So I am OK with
this here. The intent of the driver is to load a standard rbf, same
as the other Altera FPGA drivers.
There is a problem here though it will be easy to fix. This call to
revbit8x4 should happen in cyclonespi_write(), not in
cyclonespi_write_init(). The reason for that is that write_init() may
just get the first chunk of the image (the header) and that write()
will be called multiple times for the remaining chunks. The current
FPGA manager API won't show this problem since you have to give
fpga_mgr_buf_load the whole image buffer at once. But it is easy to
imagine that some time in the future we may want to expand the FPGA
manager API to support streaming where we don't have the whole buffer.
Thanks for submitting, Joshua. Will be looking at this over the
next several days.
Alan
^ 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