* [PATCH V4 0/2] vfio, platform: add ACPI support
@ 2016-05-01 21:07 Sinan Kaya
2016-05-01 21:07 ` [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset Sinan Kaya
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-05-01 21:07 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel, Sinan Kaya
The patchset makes three different changes.
1. Add support for probing ACPI platform devices.
2. Add support for ACPI _RST method support during reset
3. Make reset driver a requirement by default with an optional
kernel command line override
Sinan Kaya (2):
vfio, platform: add support for ACPI during probe and reset
vfio, platform: make reset driver a requirement by default
drivers/vfio/platform/vfio_amba.c | 5 +
drivers/vfio/platform/vfio_platform.c | 5 +
drivers/vfio/platform/vfio_platform_common.c | 184 +++++++++++++++++++++-----
drivers/vfio/platform/vfio_platform_private.h | 8 +-
4 files changed, 165 insertions(+), 37 deletions(-)
--
1.8.2.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset
2016-05-01 21:07 [PATCH V4 0/2] vfio, platform: add ACPI support Sinan Kaya
@ 2016-05-01 21:07 ` Sinan Kaya
2016-05-09 15:47 ` Eric Auger
2016-05-01 21:07 ` [PATCH V4 2/2] vfio, platform: make reset driver a requirement by default Sinan Kaya
2016-05-03 20:12 ` [PATCH V4 0/2] vfio, platform: add ACPI support Arnd Bergmann
2 siblings, 1 reply; 10+ messages in thread
From: Sinan Kaya @ 2016-05-01 21:07 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Baptiste Reynal, Alex Williamson, linux-kernel
The code is using the compatible DT string to associate a reset driver with
the actual device itself. The compatible string does not exist on ACPI
based systems. HID is the unique identifier for a device driver instead.
When ACPI is enabled, the change will query the presence of _RST method
and will call it instead of querying an external reset driver.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 176 +++++++++++++++++++++-----
drivers/vfio/platform/vfio_platform_private.h | 7 +-
2 files changed, 147 insertions(+), 36 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index e65b142..528ec04 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -13,6 +13,7 @@
*/
#include <linux/device.h>
+#include <linux/acpi.h>
#include <linux/iommu.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -41,7 +42,7 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
if (!strcmp(iter->compat, compat) &&
try_module_get(iter->owner)) {
*module = iter->owner;
- reset_fn = iter->reset;
+ reset_fn = iter->of_reset;
break;
}
}
@@ -49,20 +50,111 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
return reset_fn;
}
-static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
+
+#ifdef CONFIG_ACPI
+int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
+ struct device *dev)
{
- vdev->reset = vfio_platform_lookup_reset(vdev->compat,
- &vdev->reset_module);
- if (!vdev->reset) {
- request_module("vfio-reset:%s", vdev->compat);
- vdev->reset = vfio_platform_lookup_reset(vdev->compat,
- &vdev->reset_module);
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (acpi_disabled)
+ return -ENODEV;
+
+ if (!adev)
+ return -ENODEV;
+
+ vdev->acpihid = acpi_device_hid(adev);
+ if (!vdev->acpihid) {
+ pr_err("VFIO: cannot find ACPI HID for %s\n",
+ vdev->name);
+ return -ENODEV;
}
+ return 0;
+}
+
+static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
+{
+ struct device *dev = vdev->device;
+ acpi_handle handle = ACPI_HANDLE(dev);
+ acpi_status acpi_ret;
+ unsigned long long val;
+
+ acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
+ if (ACPI_FAILURE(acpi_ret))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
+{
+ struct device *dev = vdev->device;
+ acpi_handle handle = ACPI_HANDLE(dev);
+
+ if (acpi_has_method(handle, "_RST"))
+ return 0;
+
+ return -EINVAL;
+}
+#else
+int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ return -EINVAL;
+}
+
+static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
+{
+ return -EINVAL;
+}
+
+static int vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
+{
+ return -EINVAL;
+}
+#endif
+
+static int vfio_platform_has_reset(struct vfio_platform_device *vdev)
+{
+ if (vdev->acpihid)
+ return vfio_platform_acpi_has_reset(vdev);
+
+ if (vdev->of_reset)
+ return 0;
+
+ return -EINVAL;
+}
+
+static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
+{
+ int rc;
+
+ if (vdev->acpihid)
+ return vfio_platform_acpi_has_reset(vdev);
+
+ vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
+ &vdev->reset_module);
+ if (vdev->of_reset)
+ return 0;
+
+ rc = request_module("vfio-reset:%s", vdev->compat);
+ if (rc)
+ return rc;
+
+ vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
+ &vdev->reset_module);
+ if (vdev->of_reset)
+ return 0;
+
+ return -EINVAL;
}
static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
{
- if (vdev->reset)
+ if (vdev->acpihid)
+ return;
+
+ if (vdev->of_reset)
module_put(vdev->reset_module);
}
@@ -134,6 +226,20 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
kfree(vdev->regions);
}
+static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
+{
+ if (vdev->of_reset) {
+ dev_info(vdev->device, "reset\n");
+ vdev->of_reset(vdev);
+ return 0;
+ } else if (vdev->acpihid) {
+ return vfio_platform_acpi_call_reset(vdev);
+ }
+
+ dev_warn(vdev->device, "no reset function found!\n");
+ return -EINVAL;
+}
+
static void vfio_platform_release(void *device_data)
{
struct vfio_platform_device *vdev = device_data;
@@ -141,12 +247,7 @@ static void vfio_platform_release(void *device_data)
mutex_lock(&driver_lock);
if (!(--vdev->refcnt)) {
- if (vdev->reset) {
- dev_info(vdev->device, "reset\n");
- vdev->reset(vdev);
- } else {
- dev_warn(vdev->device, "no reset function found!\n");
- }
+ vfio_platform_call_reset(vdev);
vfio_platform_regions_cleanup(vdev);
vfio_platform_irq_cleanup(vdev);
}
@@ -175,12 +276,9 @@ static int vfio_platform_open(void *device_data)
if (ret)
goto err_irq;
- if (vdev->reset) {
- dev_info(vdev->device, "reset\n");
- vdev->reset(vdev);
- } else {
- dev_warn(vdev->device, "no reset function found!\n");
- }
+ ret = vfio_platform_call_reset(vdev);
+ if (ret)
+ goto err_irq;
}
vdev->refcnt++;
@@ -213,7 +311,7 @@ static long vfio_platform_ioctl(void *device_data,
if (info.argsz < minsz)
return -EINVAL;
- if (vdev->reset)
+ if (!vfio_platform_has_reset(vdev))
vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
info.flags = vdev->flags;
info.num_regions = vdev->num_regions;
@@ -312,10 +410,7 @@ static long vfio_platform_ioctl(void *device_data,
return ret;
} else if (cmd == VFIO_DEVICE_RESET) {
- if (vdev->reset)
- return vdev->reset(vdev);
- else
- return -EINVAL;
+ return vfio_platform_call_reset(vdev);
}
return -ENOTTY;
@@ -544,6 +639,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
.mmap = vfio_platform_mmap,
};
+int vfio_platform_of_probe(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ int ret;
+
+ ret = device_property_read_string(dev, "compatible",
+ &vdev->compat);
+ if (ret) {
+ pr_err("VFIO: cannot retrieve compat for %s\n",
+ vdev->name);
+ return ret;
+ }
+ return 0;
+}
+
int vfio_platform_probe_common(struct vfio_platform_device *vdev,
struct device *dev)
{
@@ -553,14 +663,14 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
if (!vdev)
return -EINVAL;
- ret = device_property_read_string(dev, "compatible", &vdev->compat);
- if (ret) {
- pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
- return -EINVAL;
- }
+ ret = vfio_platform_acpi_probe(vdev, dev);
+ if (ret)
+ ret = vfio_platform_of_probe(vdev, dev);
- vdev->device = dev;
+ if (ret)
+ return ret;
+ vdev->device = dev;
group = iommu_group_get(dev);
if (!group) {
pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
@@ -611,7 +721,7 @@ void vfio_platform_unregister_reset(const char *compat,
mutex_lock(&driver_lock);
list_for_each_entry_safe(iter, temp, &reset_list, link) {
- if (!strcmp(iter->compat, compat) && (iter->reset == fn)) {
+ if (!strcmp(iter->compat, compat) && (iter->of_reset == fn)) {
list_del(&iter->link);
break;
}
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 42816dd..ba9e4f8 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -58,6 +58,7 @@ struct vfio_platform_device {
struct mutex igate;
struct module *parent_module;
const char *compat;
+ const char *acpihid;
struct module *reset_module;
struct device *device;
@@ -71,7 +72,7 @@ struct vfio_platform_device {
struct resource*
(*get_resource)(struct vfio_platform_device *vdev, int i);
int (*get_irq)(struct vfio_platform_device *vdev, int i);
- int (*reset)(struct vfio_platform_device *vdev);
+ int (*of_reset)(struct vfio_platform_device *vdev);
};
typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
@@ -80,7 +81,7 @@ struct vfio_platform_reset_node {
struct list_head link;
char *compat;
struct module *owner;
- vfio_platform_reset_fn_t reset;
+ vfio_platform_reset_fn_t of_reset;
};
extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
@@ -103,7 +104,7 @@ extern void vfio_platform_unregister_reset(const char *compat,
static struct vfio_platform_reset_node __reset ## _node = { \
.owner = THIS_MODULE, \
.compat = __compat, \
- .reset = __reset, \
+ .of_reset = __reset, \
}; \
__vfio_platform_register_reset(&__reset ## _node)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH V4 2/2] vfio, platform: make reset driver a requirement by default
2016-05-01 21:07 [PATCH V4 0/2] vfio, platform: add ACPI support Sinan Kaya
2016-05-01 21:07 ` [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset Sinan Kaya
@ 2016-05-01 21:07 ` Sinan Kaya
2016-05-03 20:12 ` [PATCH V4 0/2] vfio, platform: add ACPI support Arnd Bergmann
2 siblings, 0 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-05-01 21:07 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Baptiste Reynal, Alex Williamson, linux-kernel
The code was allowing platform devices to be used without a supporting VFIO
reset driver. The hardware can be left in some inconsistent state after a
guest machine abort.
The reset driver will put the hardware back to safe state and disable
interrupts before returning the control back to the host machine.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_amba.c | 5 +++++
drivers/vfio/platform/vfio_platform.c | 5 +++++
drivers/vfio/platform/vfio_platform_common.c | 8 +++++++-
drivers/vfio/platform/vfio_platform_private.h | 1 +
4 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
index a66479b..7585902 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -23,6 +23,10 @@
#define DRIVER_AUTHOR "Antonios Motakis <a.motakis@virtualopensystems.com>"
#define DRIVER_DESC "VFIO for AMBA devices - User Level meta-driver"
+static bool reset_required = true;
+module_param(reset_required, bool, 0644);
+MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)");
+
/* probing devices from the AMBA bus */
static struct resource *get_amba_resource(struct vfio_platform_device *vdev,
@@ -68,6 +72,7 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id)
vdev->get_resource = get_amba_resource;
vdev->get_irq = get_amba_irq;
vdev->parent_module = THIS_MODULE;
+ vdev->reset_required = reset_required;
ret = vfio_platform_probe_common(vdev, &adev->dev);
if (ret) {
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index b1cc3a7..ef89146 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -23,6 +23,10 @@
#define DRIVER_AUTHOR "Antonios Motakis <a.motakis@virtualopensystems.com>"
#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver"
+static bool reset_required = true;
+module_param(reset_required, bool, 0644);
+MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)");
+
/* probing devices from the linux platform bus */
static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
@@ -66,6 +70,7 @@ static int vfio_platform_probe(struct platform_device *pdev)
vdev->get_resource = get_platform_resource;
vdev->get_irq = get_platform_irq;
vdev->parent_module = THIS_MODULE;
+ vdev->reset_required = reset_required;
ret = vfio_platform_probe_common(vdev, &pdev->dev);
if (ret)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 528ec04..b342538 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -683,7 +683,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
return ret;
}
- vfio_platform_get_reset(vdev);
+ ret = vfio_platform_get_reset(vdev);
+ if (ret && vdev->reset_required) {
+ pr_err("vfio: no reset function found for device %s\n",
+ vdev->name);
+ iommu_group_put(group);
+ return ret;
+ }
mutex_init(&vdev->igate);
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index ba9e4f8..68fbc00 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -50,6 +50,7 @@ struct vfio_platform_region {
};
struct vfio_platform_device {
+ bool reset_required;
struct vfio_platform_region *regions;
u32 num_regions;
struct vfio_platform_irq *irqs;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V4 0/2] vfio, platform: add ACPI support
2016-05-01 21:07 [PATCH V4 0/2] vfio, platform: add ACPI support Sinan Kaya
2016-05-01 21:07 ` [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset Sinan Kaya
2016-05-01 21:07 ` [PATCH V4 2/2] vfio, platform: make reset driver a requirement by default Sinan Kaya
@ 2016-05-03 20:12 ` Arnd Bergmann
2016-05-03 20:46 ` Sinan Kaya
2 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2016-05-03 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Sinan Kaya, kvm, timur, cov, jcm, eric.auger, mark.rutland,
devicetree, vikrams, marc.zyngier, linux-arm-msm, vinod.koul,
agross, shankerd
On Sunday 01 May 2016 17:07:50 Sinan Kaya wrote:
> The patchset makes three different changes.
> 1. Add support for probing ACPI platform devices.
> 2. Add support for ACPI _RST method support during reset
> 3. Make reset driver a requirement by default with an optional
> kernel command line override
Looks nice, thanks for rewriting this!
For future submissions, it would help to have a list of changes
compared to the previous versions a the patch set in the cover
letter to help reviewers, but this time my memory was still fresh
enough.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4 0/2] vfio, platform: add ACPI support
2016-05-03 20:12 ` [PATCH V4 0/2] vfio, platform: add ACPI support Arnd Bergmann
@ 2016-05-03 20:46 ` Sinan Kaya
2016-05-06 18:35 ` Sinan Kaya
0 siblings, 1 reply; 10+ messages in thread
From: Sinan Kaya @ 2016-05-03 20:46 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: kvm, timur, cov, jcm, eric.auger, mark.rutland, devicetree,
vikrams, marc.zyngier, linux-arm-msm, vinod.koul, agross,
shankerd, Rafael J. Wysocki
On 5/3/2016 4:12 PM, Arnd Bergmann wrote:
> On Sunday 01 May 2016 17:07:50 Sinan Kaya wrote:
>> The patchset makes three different changes.
>> 1. Add support for probing ACPI platform devices.
>> 2. Add support for ACPI _RST method support during reset
>> 3. Make reset driver a requirement by default with an optional
>> kernel command line override
>
> Looks nice, thanks for rewriting this!
>
> For future submissions, it would help to have a list of changes
> compared to the previous versions a the patch set in the cover
> letter to help reviewers, but this time my memory was still fresh
> enough.
>
> Arnd
>
Sure, will do. I asked Rafael to review the ACPI code.
+ ret = vfio_platform_call_reset(vdev);
This is the only problem I'm seeing in the code. I tested the ACPI path only.
I'm wondering if Eric could test the device tree version.
+ if (ret)
+ goto err_irq;
I'll get rid of this line on the first patch and move it to second
patch along with reset_required check.
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4 0/2] vfio, platform: add ACPI support
2016-05-03 20:46 ` Sinan Kaya
@ 2016-05-06 18:35 ` Sinan Kaya
2016-05-06 19:54 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Sinan Kaya @ 2016-05-06 18:35 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel, eric.auger
Cc: kvm, timur, cov, jcm, mark.rutland, devicetree, vikrams,
marc.zyngier, linux-arm-msm, vinod.koul, agross, shankerd,
Rafael J. Wysocki
On 5/3/2016 4:46 PM, Sinan Kaya wrote:
> On 5/3/2016 4:12 PM, Arnd Bergmann wrote:
>> On Sunday 01 May 2016 17:07:50 Sinan Kaya wrote:
>>> The patchset makes three different changes.
>>> 1. Add support for probing ACPI platform devices.
>>> 2. Add support for ACPI _RST method support during reset
>>> 3. Make reset driver a requirement by default with an optional
>>> kernel command line override
>>
>> Looks nice, thanks for rewriting this!
>>
>> For future submissions, it would help to have a list of changes
>> compared to the previous versions a the patch set in the cover
>> letter to help reviewers, but this time my memory was still fresh
>> enough.
>>
>> Arnd
>>
>
> Sure, will do. I asked Rafael to review the ACPI code.
>
> + ret = vfio_platform_call_reset(vdev);
>
> This is the only problem I'm seeing in the code. I tested the ACPI path only.
> I'm wondering if Eric could test the device tree version.
>
> + if (ret)
> + goto err_irq;
>
> I'll get rid of this line on the first patch and move it to second
> patch along with reset_required check.
>
Before I post my next patch, I wanted to see if there is any other
feedback. Rafael mentioned to me that this code should go through
his branch as it is ACPI specific.
Are we OK with this? Alex?
I'm hoping that Eric Auger will be able to test DT in parallel.
I haven't heart from him though.
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4 0/2] vfio, platform: add ACPI support
2016-05-06 19:54 ` Rafael J. Wysocki
@ 2016-05-06 19:52 ` Sinan Kaya
0 siblings, 0 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-05-06 19:52 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Arnd Bergmann, linux-arm-kernel, eric.auger, kvm, timur, cov,
jcm, mark.rutland, devicetree, vikrams, marc.zyngier,
linux-arm-msm, vinod.koul, agross, shankerd
On 5/6/2016 3:54 PM, Rafael J. Wysocki wrote:
>> Before I post my next patch, I wanted to see if there is any other
>> > feedback. Rafael mentioned to me that this code should go through
>> > his branch as it is ACPI specific.
> It doesn't have to, but it should be posted to linux-acpi@vger.kernel.org at
> least for a review in there.
>
I'll do that on the next post.
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4 0/2] vfio, platform: add ACPI support
2016-05-06 18:35 ` Sinan Kaya
@ 2016-05-06 19:54 ` Rafael J. Wysocki
2016-05-06 19:52 ` Sinan Kaya
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-05-06 19:54 UTC (permalink / raw)
To: Sinan Kaya
Cc: Arnd Bergmann, linux-arm-kernel, eric.auger, kvm, timur, cov,
jcm, mark.rutland, devicetree, vikrams, marc.zyngier,
linux-arm-msm, vinod.koul, agross, shankerd
On Friday, May 06, 2016 02:35:45 PM Sinan Kaya wrote:
> On 5/3/2016 4:46 PM, Sinan Kaya wrote:
> > On 5/3/2016 4:12 PM, Arnd Bergmann wrote:
> >> On Sunday 01 May 2016 17:07:50 Sinan Kaya wrote:
> >>> The patchset makes three different changes.
> >>> 1. Add support for probing ACPI platform devices.
> >>> 2. Add support for ACPI _RST method support during reset
> >>> 3. Make reset driver a requirement by default with an optional
> >>> kernel command line override
> >>
> >> Looks nice, thanks for rewriting this!
> >>
> >> For future submissions, it would help to have a list of changes
> >> compared to the previous versions a the patch set in the cover
> >> letter to help reviewers, but this time my memory was still fresh
> >> enough.
> >>
> >> Arnd
> >>
> >
> > Sure, will do. I asked Rafael to review the ACPI code.
> >
> > + ret = vfio_platform_call_reset(vdev);
> >
> > This is the only problem I'm seeing in the code. I tested the ACPI path only.
> > I'm wondering if Eric could test the device tree version.
> >
> > + if (ret)
> > + goto err_irq;
> >
> > I'll get rid of this line on the first patch and move it to second
> > patch along with reset_required check.
> >
>
> Before I post my next patch, I wanted to see if there is any other
> feedback. Rafael mentioned to me that this code should go through
> his branch as it is ACPI specific.
It doesn't have to, but it should be posted to linux-acpi@vger.kernel.org at
least for a review in there.
> Are we OK with this? Alex?
>
> I'm hoping that Eric Auger will be able to test DT in parallel.
> I haven't heart from him though.
Surely heard?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset
2016-05-01 21:07 ` [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset Sinan Kaya
@ 2016-05-09 15:47 ` Eric Auger
2016-05-09 19:45 ` Sinan Kaya
0 siblings, 1 reply; 10+ messages in thread
From: Eric Auger @ 2016-05-09 15:47 UTC (permalink / raw)
To: Sinan Kaya, kvm, timur, cov, jcm
Cc: mark.rutland, devicetree, Baptiste Reynal, vikrams, marc.zyngier,
linux-arm-msm, linux-kernel, vinod.koul, Alex Williamson,
linux-arm-kernel, agross, shankerd
Hi Sinan,
On 05/01/2016 11:07 PM, Sinan Kaya wrote:
> The code is using the compatible DT string to associate a reset driver with
> the actual device itself. The compatible string does not exist on ACPI
> based systems. HID is the unique identifier for a device driver instead.
>
> When ACPI is enabled, the change will query the presence of _RST method
> and will call it instead of querying an external reset driver.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 176 +++++++++++++++++++++-----
> drivers/vfio/platform/vfio_platform_private.h | 7 +-
> 2 files changed, 147 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index e65b142..528ec04 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -13,6 +13,7 @@
> */
>
> #include <linux/device.h>
> +#include <linux/acpi.h>
> #include <linux/iommu.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> @@ -41,7 +42,7 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
> if (!strcmp(iter->compat, compat) &&
> try_module_get(iter->owner)) {
> *module = iter->owner;
> - reset_fn = iter->reset;
> + reset_fn = iter->of_reset;
> break;
> }
> }
> @@ -49,20 +50,111 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
> return reset_fn;
> }
>
> -static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> +
> +#ifdef CONFIG_ACPI
> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> + struct device *dev)
> {
> - vdev->reset = vfio_platform_lookup_reset(vdev->compat,
> - &vdev->reset_module);
> - if (!vdev->reset) {
> - request_module("vfio-reset:%s", vdev->compat);
> - vdev->reset = vfio_platform_lookup_reset(vdev->compat,
> - &vdev->reset_module);
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (acpi_disabled)
> + return -ENODEV;
> +
> + if (!adev)
> + return -ENODEV;
> +
> + vdev->acpihid = acpi_device_hid(adev);
> + if (!vdev->acpihid) {
> + pr_err("VFIO: cannot find ACPI HID for %s\n",
> + vdev->name);
> + return -ENODEV;
> }
> + return 0;
> +}
> +
> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
> +{
> + struct device *dev = vdev->device;
> + acpi_handle handle = ACPI_HANDLE(dev);
> + acpi_status acpi_ret;
> + unsigned long long val;
> +
> + acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
> + if (ACPI_FAILURE(acpi_ret))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
> +{
> + struct device *dev = vdev->device;
> + acpi_handle handle = ACPI_HANDLE(dev);
> +
> + if (acpi_has_method(handle, "_RST"))
> + return 0;
Can't you directly return acpi_has_method(handle, "_RST"), hence return
a boolean?
> +
> + return -EINVAL;
> +}
> +#else
> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> + struct device *dev)
> +{
> + return -EINVAL;
> +}
> +
> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
> +{
> + return -EINVAL;
> +}
> +
> +static int vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +static int vfio_platform_has_reset(struct vfio_platform_device *vdev)
> +{
return a boolean?
> + if (vdev->acpihid)
> + return vfio_platform_acpi_has_reset(vdev);
> +
> + if (vdev->of_reset)
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
> +{
> + int rc;
> +
> + if (vdev->acpihid)
> + return vfio_platform_acpi_has_reset(vdev);
> +
> + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> + &vdev->reset_module);
> + if (vdev->of_reset)
> + return 0;
> +
> + rc = request_module("vfio-reset:%s", vdev->compat);
> + if (rc)
> + return rc;
> +
> + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> + &vdev->reset_module);
> + if (vdev->of_reset)
> + return 0;
> +
> + return -EINVAL;
> }
>
> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> {
> - if (vdev->reset)
> + if (vdev->acpihid)
> + return;
> +
> + if (vdev->of_reset)
> module_put(vdev->reset_module);
> }
>
> @@ -134,6 +226,20 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
> kfree(vdev->regions);
> }
>
> +static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
> +{
> + if (vdev->of_reset) {
> + dev_info(vdev->device, "reset\n");
> + vdev->of_reset(vdev);
return vdev->of_reset(vdev) to align with acpi reset behavior.
> + return 0;
> + } else if (vdev->acpihid) {
> + return vfio_platform_acpi_call_reset(vdev);
I think it would make sense to have the same dev_info traces for dt and
acpi.
> + }
> +
> + dev_warn(vdev->device, "no reset function found!\n");
> + return -EINVAL;
> +}
> +
> static void vfio_platform_release(void *device_data)
> {
> struct vfio_platform_device *vdev = device_data;
> @@ -141,12 +247,7 @@ static void vfio_platform_release(void *device_data)
> mutex_lock(&driver_lock);
>
> if (!(--vdev->refcnt)) {
> - if (vdev->reset) {
> - dev_info(vdev->device, "reset\n");
> - vdev->reset(vdev);
> - } else {
> - dev_warn(vdev->device, "no reset function found!\n");
> - }
> + vfio_platform_call_reset(vdev);
> vfio_platform_regions_cleanup(vdev);
> vfio_platform_irq_cleanup(vdev);
> }
> @@ -175,12 +276,9 @@ static int vfio_platform_open(void *device_data)
> if (ret)
> goto err_irq;
>
> - if (vdev->reset) {
> - dev_info(vdev->device, "reset\n");
> - vdev->reset(vdev);
> - } else {
> - dev_warn(vdev->device, "no reset function found!\n");
> - }
> + ret = vfio_platform_call_reset(vdev);
> + if (ret)
> + goto err_irq;
This change should be in next patch since if you fail finding a reset
function, you clean things up, mandating a reset function to exist.
Also in case the reset function fails you change the behavior which is
not detailed in the commit msg.
> }
>
> vdev->refcnt++;
> @@ -213,7 +311,7 @@ static long vfio_platform_ioctl(void *device_data,
> if (info.argsz < minsz)
> return -EINVAL;
>
> - if (vdev->reset)
> + if (!vfio_platform_has_reset(vdev))
> vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
> info.flags = vdev->flags;
> info.num_regions = vdev->num_regions;
> @@ -312,10 +410,7 @@ static long vfio_platform_ioctl(void *device_data,
> return ret;
>
> } else if (cmd == VFIO_DEVICE_RESET) {
> - if (vdev->reset)
> - return vdev->reset(vdev);
> - else
> - return -EINVAL;
> + return vfio_platform_call_reset(vdev);
Here you also change the behavior: before, in case the dt reset failed
we returned an error. Now we return 0.
To me this patch would deserve to be split into 2 parts: ACPI probing
without reset and then ACPI reset. In case you change the behavior of
existing dt reset, please put that in a separate patch.
Best Regards
Eric
> }
>
> return -ENOTTY;
> @@ -544,6 +639,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
> .mmap = vfio_platform_mmap,
> };
>
> +int vfio_platform_of_probe(struct vfio_platform_device *vdev,
> + struct device *dev)
> +{
> + int ret;
> +
> + ret = device_property_read_string(dev, "compatible",
> + &vdev->compat);
> + if (ret) {
> + pr_err("VFIO: cannot retrieve compat for %s\n",
> + vdev->name);
> + return ret;
> + }
> + return 0;
> +}
> +
> int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> struct device *dev)
> {
> @@ -553,14 +663,14 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> if (!vdev)
> return -EINVAL;
>
> - ret = device_property_read_string(dev, "compatible", &vdev->compat);
> - if (ret) {
> - pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
> - return -EINVAL;
> - }
> + ret = vfio_platform_acpi_probe(vdev, dev);
> + if (ret)
> + ret = vfio_platform_of_probe(vdev, dev);
>
> - vdev->device = dev;
> + if (ret)
> + return ret;
>
> + vdev->device = dev;
> group = iommu_group_get(dev);
> if (!group) {
> pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
> @@ -611,7 +721,7 @@ void vfio_platform_unregister_reset(const char *compat,
>
> mutex_lock(&driver_lock);
> list_for_each_entry_safe(iter, temp, &reset_list, link) {
> - if (!strcmp(iter->compat, compat) && (iter->reset == fn)) {
> + if (!strcmp(iter->compat, compat) && (iter->of_reset == fn)) {
> list_del(&iter->link);
> break;
> }
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 42816dd..ba9e4f8 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -58,6 +58,7 @@ struct vfio_platform_device {
> struct mutex igate;
> struct module *parent_module;
> const char *compat;
> + const char *acpihid;
> struct module *reset_module;
> struct device *device;
>
> @@ -71,7 +72,7 @@ struct vfio_platform_device {
> struct resource*
> (*get_resource)(struct vfio_platform_device *vdev, int i);
> int (*get_irq)(struct vfio_platform_device *vdev, int i);
> - int (*reset)(struct vfio_platform_device *vdev);
> + int (*of_reset)(struct vfio_platform_device *vdev);
> };
>
> typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
> @@ -80,7 +81,7 @@ struct vfio_platform_reset_node {
> struct list_head link;
> char *compat;
> struct module *owner;
> - vfio_platform_reset_fn_t reset;
> + vfio_platform_reset_fn_t of_reset;
> };
>
> extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> @@ -103,7 +104,7 @@ extern void vfio_platform_unregister_reset(const char *compat,
> static struct vfio_platform_reset_node __reset ## _node = { \
> .owner = THIS_MODULE, \
> .compat = __compat, \
> - .reset = __reset, \
> + .of_reset = __reset, \
> }; \
> __vfio_platform_register_reset(&__reset ## _node)
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset
2016-05-09 15:47 ` Eric Auger
@ 2016-05-09 19:45 ` Sinan Kaya
0 siblings, 0 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-05-09 19:45 UTC (permalink / raw)
To: Eric Auger, kvm, timur, cov, jcm
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Baptiste Reynal, Alex Williamson, linux-kernel
On 5/9/2016 11:47 AM, Eric Auger wrote:
> Hi Sinan,
> On 05/01/2016 11:07 PM, Sinan Kaya wrote:
>> The code is using the compatible DT string to associate a reset driver with
>> the actual device itself. The compatible string does not exist on ACPI
>> based systems. HID is the unique identifier for a device driver instead.
>>
>> When ACPI is enabled, the change will query the presence of _RST method
>> and will call it instead of querying an external reset driver.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 176 +++++++++++++++++++++-----
>> drivers/vfio/platform/vfio_platform_private.h | 7 +-
>> 2 files changed, 147 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index e65b142..528ec04 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -13,6 +13,7 @@
>> */
>>
>> #include <linux/device.h>
>> +#include <linux/acpi.h>
>> #include <linux/iommu.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> @@ -41,7 +42,7 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
>> if (!strcmp(iter->compat, compat) &&
>> try_module_get(iter->owner)) {
>> *module = iter->owner;
>> - reset_fn = iter->reset;
>> + reset_fn = iter->of_reset;
>> break;
>> }
>> }
>> @@ -49,20 +50,111 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
>> return reset_fn;
>> }
>>
>> -static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
>> +
>> +#ifdef CONFIG_ACPI
>> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
>> {
>> - vdev->reset = vfio_platform_lookup_reset(vdev->compat,
>> - &vdev->reset_module);
>> - if (!vdev->reset) {
>> - request_module("vfio-reset:%s", vdev->compat);
>> - vdev->reset = vfio_platform_lookup_reset(vdev->compat,
>> - &vdev->reset_module);
>> + struct acpi_device *adev = ACPI_COMPANION(dev);
>> +
>> + if (acpi_disabled)
>> + return -ENODEV;
>> +
>> + if (!adev)
>> + return -ENODEV;
>> +
>> + vdev->acpihid = acpi_device_hid(adev);
>> + if (!vdev->acpihid) {
>> + pr_err("VFIO: cannot find ACPI HID for %s\n",
>> + vdev->name);
>> + return -ENODEV;
>> }
>> + return 0;
>> +}
>> +
>> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
>> +{
>> + struct device *dev = vdev->device;
>> + acpi_handle handle = ACPI_HANDLE(dev);
>> + acpi_status acpi_ret;
>> + unsigned long long val;
>> +
>> + acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
>> + if (ACPI_FAILURE(acpi_ret))
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static int vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
>> +{
>> + struct device *dev = vdev->device;
>> + acpi_handle handle = ACPI_HANDLE(dev);
>> +
>> + if (acpi_has_method(handle, "_RST"))
>> + return 0;
> Can't you directly return acpi_has_method(handle, "_RST"), hence return
> a boolean?
Makes sense. I'll do that on the next post.
>> +
>> + return -EINVAL;
>> +}
>> +#else
>> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +static int vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
>> +{
>> + return -EINVAL;
>> +}
>> +#endif
>> +
>> +static int vfio_platform_has_reset(struct vfio_platform_device *vdev)
>> +{
> return a boolean?
OK
>> + if (vdev->acpihid)
>> + return vfio_platform_acpi_has_reset(vdev);
>> +
>> + if (vdev->of_reset)
>> + return 0;
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
>> +{
>> + int rc;
>> +
>> + if (vdev->acpihid)
>> + return vfio_platform_acpi_has_reset(vdev);
>> +
>> + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
>> + &vdev->reset_module);
>> + if (vdev->of_reset)
>> + return 0;
>> +
>> + rc = request_module("vfio-reset:%s", vdev->compat);
>> + if (rc)
>> + return rc;
>> +
>> + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
>> + &vdev->reset_module);
>> + if (vdev->of_reset)
>> + return 0;
>> +
>> + return -EINVAL;
>> }
>>
>> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
>> {
>> - if (vdev->reset)
>> + if (vdev->acpihid)
>> + return;
>> +
>> + if (vdev->of_reset)
>> module_put(vdev->reset_module);
>> }
>>
>> @@ -134,6 +226,20 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
>> kfree(vdev->regions);
>> }
>>
>> +static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
>> +{
>> + if (vdev->of_reset) {
>> + dev_info(vdev->device, "reset\n");
>> + vdev->of_reset(vdev);
> return vdev->of_reset(vdev) to align with acpi reset behavior.
OK. I didn't realize it was returning a value.
>> + return 0;
>> + } else if (vdev->acpihid) {
>> + return vfio_platform_acpi_call_reset(vdev);
> I think it would make sense to have the same dev_info traces for dt and
> acpi.
>> + }
>> +
>> + dev_warn(vdev->device, "no reset function found!\n");
>> + return -EINVAL;
>> +}
>> +
>> static void vfio_platform_release(void *device_data)
>> {
>> struct vfio_platform_device *vdev = device_data;
>> @@ -141,12 +247,7 @@ static void vfio_platform_release(void *device_data)
>> mutex_lock(&driver_lock);
>>
>> if (!(--vdev->refcnt)) {
>> - if (vdev->reset) {
>> - dev_info(vdev->device, "reset\n");
>> - vdev->reset(vdev);
>> - } else {
>> - dev_warn(vdev->device, "no reset function found!\n");
>> - }
>> + vfio_platform_call_reset(vdev);
>> vfio_platform_regions_cleanup(vdev);
>> vfio_platform_irq_cleanup(vdev);
>> }
>> @@ -175,12 +276,9 @@ static int vfio_platform_open(void *device_data)
>> if (ret)
>> goto err_irq;
>>
>> - if (vdev->reset) {
>> - dev_info(vdev->device, "reset\n");
>> - vdev->reset(vdev);
>> - } else {
>> - dev_warn(vdev->device, "no reset function found!\n");
>> - }
>> + ret = vfio_platform_call_reset(vdev);
>> + if (ret)
>> + goto err_irq;
> This change should be in next patch since if you fail finding a reset
> function, you clean things up, mandating a reset function to exist.
>
> Also in case the reset function fails you change the behavior which is
> not detailed in the commit msg.
I admitted this on another patch. Yes, I'll fix this.
>> }
>>
>> vdev->refcnt++;
>> @@ -213,7 +311,7 @@ static long vfio_platform_ioctl(void *device_data,
>> if (info.argsz < minsz)
>> return -EINVAL;
>>
>> - if (vdev->reset)
>> + if (!vfio_platform_has_reset(vdev))
>> vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
>> info.flags = vdev->flags;
>> info.num_regions = vdev->num_regions;
>> @@ -312,10 +410,7 @@ static long vfio_platform_ioctl(void *device_data,
>> return ret;
>>
>> } else if (cmd == VFIO_DEVICE_RESET) {
>> - if (vdev->reset)
>> - return vdev->reset(vdev);
>> - else
>> - return -EINVAL;
>> + return vfio_platform_call_reset(vdev);
> Here you also change the behavior: before, in case the dt reset failed
> we returned an error. Now we return 0.
Yes, I'll fix it.
>
> To me this patch would deserve to be split into 2 parts: ACPI probing
> without reset and then ACPI reset. In case you change the behavior of
> existing dt reset, please put that in a separate patch.
Sure, I'll move vfio_platform_probe_common changes to a seperate patch.
I'll try to break it as much as I can before the post.
>
> Best Regards
>
> Eric
>> }
>>
>> return -ENOTTY;
>> @@ -544,6 +639,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
>> .mmap = vfio_platform_mmap,
>> };
>>
>> +int vfio_platform_of_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
>> +{
>> + int ret;
>> +
>> + ret = device_property_read_string(dev, "compatible",
>> + &vdev->compat);
>> + if (ret) {
>> + pr_err("VFIO: cannot retrieve compat for %s\n",
>> + vdev->name);
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> struct device *dev)
>> {
>> @@ -553,14 +663,14 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> if (!vdev)
>> return -EINVAL;
>>
>> - ret = device_property_read_string(dev, "compatible", &vdev->compat);
>> - if (ret) {
>> - pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
>> - return -EINVAL;
>> - }
>> + ret = vfio_platform_acpi_probe(vdev, dev);
>> + if (ret)
>> + ret = vfio_platform_of_probe(vdev, dev);
>>
>> - vdev->device = dev;
>> + if (ret)
>> + return ret;
>>
>> + vdev->device = dev;
>> group = iommu_group_get(dev);
>> if (!group) {
>> pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
>> @@ -611,7 +721,7 @@ void vfio_platform_unregister_reset(const char *compat,
>>
>> mutex_lock(&driver_lock);
>> list_for_each_entry_safe(iter, temp, &reset_list, link) {
>> - if (!strcmp(iter->compat, compat) && (iter->reset == fn)) {
>> + if (!strcmp(iter->compat, compat) && (iter->of_reset == fn)) {
>> list_del(&iter->link);
>> break;
>> }
>> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
>> index 42816dd..ba9e4f8 100644
>> --- a/drivers/vfio/platform/vfio_platform_private.h
>> +++ b/drivers/vfio/platform/vfio_platform_private.h
>> @@ -58,6 +58,7 @@ struct vfio_platform_device {
>> struct mutex igate;
>> struct module *parent_module;
>> const char *compat;
>> + const char *acpihid;
>> struct module *reset_module;
>> struct device *device;
>>
>> @@ -71,7 +72,7 @@ struct vfio_platform_device {
>> struct resource*
>> (*get_resource)(struct vfio_platform_device *vdev, int i);
>> int (*get_irq)(struct vfio_platform_device *vdev, int i);
>> - int (*reset)(struct vfio_platform_device *vdev);
>> + int (*of_reset)(struct vfio_platform_device *vdev);
>> };
>>
>> typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
>> @@ -80,7 +81,7 @@ struct vfio_platform_reset_node {
>> struct list_head link;
>> char *compat;
>> struct module *owner;
>> - vfio_platform_reset_fn_t reset;
>> + vfio_platform_reset_fn_t of_reset;
>> };
>>
>> extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> @@ -103,7 +104,7 @@ extern void vfio_platform_unregister_reset(const char *compat,
>> static struct vfio_platform_reset_node __reset ## _node = { \
>> .owner = THIS_MODULE, \
>> .compat = __compat, \
>> - .reset = __reset, \
>> + .of_reset = __reset, \
>> }; \
>> __vfio_platform_register_reset(&__reset ## _node)
>>
>>
>
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-05-09 19:45 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-01 21:07 [PATCH V4 0/2] vfio, platform: add ACPI support Sinan Kaya
2016-05-01 21:07 ` [PATCH V4 1/2] vfio, platform: add support for ACPI during probe and reset Sinan Kaya
2016-05-09 15:47 ` Eric Auger
2016-05-09 19:45 ` Sinan Kaya
2016-05-01 21:07 ` [PATCH V4 2/2] vfio, platform: make reset driver a requirement by default Sinan Kaya
2016-05-03 20:12 ` [PATCH V4 0/2] vfio, platform: add ACPI support Arnd Bergmann
2016-05-03 20:46 ` Sinan Kaya
2016-05-06 18:35 ` Sinan Kaya
2016-05-06 19:54 ` Rafael J. Wysocki
2016-05-06 19:52 ` Sinan Kaya
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).