* [PATCH V2 1/3] Documentation: DT: qcom_hidma: bump HW revision for the bugfixed HW
@ 2017-11-08 16:29 Sinan Kaya
2017-11-08 16:29 ` [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision Sinan Kaya
2017-11-08 16:29 ` [PATCH V2 3/3] dmaengine: qcom_hidma: add identity register support Sinan Kaya
0 siblings, 2 replies; 12+ messages in thread
From: Sinan Kaya @ 2017-11-08 16:29 UTC (permalink / raw)
To: dmaengine, timur
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, linux-kernel
A new version of the HIDMA IP has been released with bug fixes. Bumping the
hardware version to differentiate from others.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
index 55492c2..5d93d6d 100644
--- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
@@ -47,8 +47,8 @@ When the OS is not in control of the management interface (i.e. it's a guest),
the channel nodes appear on their own, not under a management node.
Required properties:
-- compatible: must contain "qcom,hidma-1.0" for initial HW or "qcom,hidma-1.1"
-for MSI capable HW.
+- compatible: must contain "qcom,hidma-1.0" for initial HW or
+ "qcom,hidma-1.1"/"qcom,hidma-1.2" for MSI capable HW.
- reg: Addresses for the transfer and event channel
- interrupts: Should contain the event interrupt
- desc-count: Number of asynchronous requests this channel can handle
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 16:29 [PATCH V2 1/3] Documentation: DT: qcom_hidma: bump HW revision for the bugfixed HW Sinan Kaya
@ 2017-11-08 16:29 ` Sinan Kaya
2017-11-08 16:49 ` Timur Tabi
2017-11-08 17:51 ` Robin Murphy
2017-11-08 16:29 ` [PATCH V2 3/3] dmaengine: qcom_hidma: add identity register support Sinan Kaya
1 sibling, 2 replies; 12+ messages in thread
From: Sinan Kaya @ 2017-11-08 16:29 UTC (permalink / raw)
To: dmaengine, timur
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, linux-kernel
Add support for probing the newer HW and also organize MSI capable hardware
into an array for maintenance reasons.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/dma/qcom/hidma.c | 41 +++++++++++++++++++++++++++++------------
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index e366985..4ef7d6f 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -50,6 +50,7 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/of_dma.h>
+#include <linux/of_device.h>
#include <linux/property.h>
#include <linux/delay.h>
#include <linux/acpi.h>
@@ -104,6 +105,26 @@ static void hidma_free(struct hidma_dev *dmadev)
module_param(nr_desc_prm, uint, 0644);
MODULE_PARM_DESC(nr_desc_prm, "number of descriptors (default: 0)");
+#define HIDMA_MAX_DEV_MATCH 10
+
+struct hidma_cap {
+ const struct of_device_id of[HIDMA_MAX_DEV_MATCH];
+ const struct acpi_device_id acpi[HIDMA_MAX_DEV_MATCH];
+};
+
+static struct hidma_cap hidma_msi_cap = {
+ .of = {
+ {.compatible = "qcom,hidma-1.1",},
+ {.compatible = "qcom,hidma-1.2",},
+ {},
+ },
+
+ .acpi = {
+ {"QCOM8062"},
+ {"QCOM8063"},
+ {},
+ }
+};
/* process completed descriptors */
static void hidma_process_completed(struct hidma_chan *mchan)
@@ -739,22 +760,16 @@ static int hidma_request_msi(struct hidma_dev *dmadev,
static bool hidma_msi_capable(struct device *dev)
{
struct acpi_device *adev = ACPI_COMPANION(dev);
- const char *of_compat;
- int ret = -EINVAL;
-
- if (!adev || acpi_disabled) {
- ret = device_property_read_string(dev, "compatible",
- &of_compat);
- if (ret)
- return false;
+ int ret;
- ret = strcmp(of_compat, "qcom,hidma-1.1");
- } else {
+ if (!adev || acpi_disabled)
+ ret = of_match_device(hidma_msi_cap.of, dev) != NULL;
+ else {
#ifdef CONFIG_ACPI
- ret = strcmp(acpi_device_hid(adev), "QCOM8062");
+ ret = acpi_match_device(hidma_msi_cap.acpi, dev) != NULL;
#endif
}
- return ret == 0;
+ return ret;
}
static int hidma_probe(struct platform_device *pdev)
@@ -954,6 +969,7 @@ static int hidma_remove(struct platform_device *pdev)
static const struct acpi_device_id hidma_acpi_ids[] = {
{"QCOM8061"},
{"QCOM8062"},
+ {"QCOM8063"},
{},
};
MODULE_DEVICE_TABLE(acpi, hidma_acpi_ids);
@@ -962,6 +978,7 @@ static int hidma_remove(struct platform_device *pdev)
static const struct of_device_id hidma_match[] = {
{.compatible = "qcom,hidma-1.0",},
{.compatible = "qcom,hidma-1.1",},
+ {.compatible = "qcom,hidma-1.2",},
{},
};
MODULE_DEVICE_TABLE(of, hidma_match);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2 3/3] dmaengine: qcom_hidma: add identity register support
2017-11-08 16:29 [PATCH V2 1/3] Documentation: DT: qcom_hidma: bump HW revision for the bugfixed HW Sinan Kaya
2017-11-08 16:29 ` [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision Sinan Kaya
@ 2017-11-08 16:29 ` Sinan Kaya
1 sibling, 0 replies; 12+ messages in thread
From: Sinan Kaya @ 2017-11-08 16:29 UTC (permalink / raw)
To: dmaengine, timur
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, linux-kernel
The location for destination event channel register has been relocated from
offset 0x28 to 0x40. Update the code accordingly.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/dma/qcom/hidma.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index 4ef7d6f..e8b7be0 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -126,6 +126,18 @@ struct hidma_cap {
}
};
+static struct hidma_cap hidma_identity_cap = {
+ .of = {
+ {.compatible = "qcom,hidma-1.2",},
+ {},
+ },
+
+ .acpi = {
+ {"QCOM8063"},
+ {},
+ }
+};
+
/* process completed descriptors */
static void hidma_process_completed(struct hidma_chan *mchan)
{
@@ -772,6 +784,22 @@ static bool hidma_msi_capable(struct device *dev)
return ret;
}
+static bool hidma_identity_capable(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ int ret;
+
+ if (!adev || acpi_disabled)
+ ret = of_match_device(hidma_identity_cap.of, dev) != NULL;
+ else {
+#ifdef CONFIG_ACPI
+ ret = acpi_match_device(hidma_identity_cap.acpi, dev) != NULL;
+#endif
+ }
+
+ return ret;
+}
+
static int hidma_probe(struct platform_device *pdev)
{
struct hidma_dev *dmadev;
@@ -863,7 +891,10 @@ static int hidma_probe(struct platform_device *pdev)
if (!dmadev->nr_descriptors)
dmadev->nr_descriptors = HIDMA_NR_DEFAULT_DESC;
- dmadev->chidx = readl(dmadev->dev_trca + 0x28);
+ if (hidma_identity_capable(&pdev->dev))
+ dmadev->chidx = readl(dmadev->dev_trca + 0x40);
+ else
+ dmadev->chidx = readl(dmadev->dev_trca + 0x28);
/* Set DMA mask to 64 bits. */
rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 16:29 ` [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision Sinan Kaya
@ 2017-11-08 16:49 ` Timur Tabi
2017-11-08 16:58 ` Sinan Kaya
2017-11-08 17:51 ` Robin Murphy
1 sibling, 1 reply; 12+ messages in thread
From: Timur Tabi @ 2017-11-08 16:49 UTC (permalink / raw)
To: Sinan Kaya, dmaengine; +Cc: linux-arm-msm, linux-arm-kernel, linux-kernel
On 11/08/2017 10:29 AM, Sinan Kaya wrote:
> +#define HIDMA_MAX_DEV_MATCH 10
> +
> +struct hidma_cap {
> + const struct of_device_id of[HIDMA_MAX_DEV_MATCH];
> + const struct acpi_device_id acpi[HIDMA_MAX_DEV_MATCH];
> +};
This seems wrong. You're defining an array of size 10, but it only has
three elements. And the third element is a sentinel, which is typically
used to avoid specifying the size of the array.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 16:49 ` Timur Tabi
@ 2017-11-08 16:58 ` Sinan Kaya
2017-11-08 17:12 ` Timur Tabi
0 siblings, 1 reply; 12+ messages in thread
From: Sinan Kaya @ 2017-11-08 16:58 UTC (permalink / raw)
To: Timur Tabi, dmaengine; +Cc: linux-arm-msm, linux-arm-kernel, linux-kernel
On 11/8/2017 11:49 AM, Timur Tabi wrote:
> On 11/08/2017 10:29 AM, Sinan Kaya wrote:
>> +#define HIDMA_MAX_DEV_MATCH 10
>> +
>> +struct hidma_cap {
>> + const struct of_device_id of[HIDMA_MAX_DEV_MATCH];
>> + const struct acpi_device_id acpi[HIDMA_MAX_DEV_MATCH];
>> +};
>
> This seems wrong. You're defining an array of size 10, but it only has three elements. And the third element is a sentinel, which is typically used to avoid specifying the size of the array.
>
It is true that I define an array of size 10. It is just some arbitrary number
with forward thinking. I don't really have to use it to the maximum today as long
as I satisfy the calling conventions.
What is important is that the of_device_match() and acpi_device_match() functions
will stop searching only if they find an empty element. The {} element.
That's the calling semantics of of_device_match() and acpi_device_match().
Besides, C compiler also won't let me put two arrays together like this.
struct my_struct {
struct some_struct array1[]
struct some_struct array2[]
}
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 16:58 ` Sinan Kaya
@ 2017-11-08 17:12 ` Timur Tabi
2017-11-08 17:37 ` Sinan Kaya
0 siblings, 1 reply; 12+ messages in thread
From: Timur Tabi @ 2017-11-08 17:12 UTC (permalink / raw)
To: Sinan Kaya, dmaengine; +Cc: linux-arm-msm, linux-arm-kernel, linux-kernel
On 11/08/2017 10:58 AM, Sinan Kaya wrote:
> Besides, C compiler also won't let me put two arrays together like this.
>
> struct my_struct {
> struct some_struct array1[]
> struct some_struct array2[]
> }
Why not this:
const struct of_device_id hidma_msi_of_ids[] = {
{.compatible = "qcom,hidma-1.1",},
{.compatible = "qcom,hidma-1.2",},
{},
},
static const struct acpi_device_id hidma_msi_acpi_ids[] = {
{"QCOM8001", QDF2XXX_V1},
{"QCOM8002", QDF2XXX_V2},
{},
};
struct hidma_cap {
const struct of_device_id *of;
const struct acpi_device_id *acpi;
};
static struct hidma_cap hidma_msi_cap = {
hidma_msi_of_ids,
hidma_msi_acpi_ids
}
Keep in mind that you also need to add MODULE_DEVICE_TABLE entries,
which you can't really do with your approach.
MODULE_DEVICE_TABLE(of, hidma_msi_of_ids);
MODULE_DEVICE_TABLE(acpi, hidma_msi_acpi_ids);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 17:12 ` Timur Tabi
@ 2017-11-08 17:37 ` Sinan Kaya
2017-11-08 17:51 ` Timur Tabi
0 siblings, 1 reply; 12+ messages in thread
From: Sinan Kaya @ 2017-11-08 17:37 UTC (permalink / raw)
To: Timur Tabi, dmaengine; +Cc: linux-arm-msm, linux-arm-kernel, linux-kernel
On 11/8/2017 12:12 PM, Timur Tabi wrote:
> On 11/08/2017 10:58 AM, Sinan Kaya wrote:
>> Besides, C compiler also won't let me put two arrays together like this.
>>
>> struct my_struct {
>> struct some_struct array1[]
>> struct some_struct array2[]
>> }
>
> Why not this:
>
> const struct of_device_id hidma_msi_of_ids[] = {
> {.compatible = "qcom,hidma-1.1",},
> {.compatible = "qcom,hidma-1.2",},
> {},
> },
>
> static const struct acpi_device_id hidma_msi_acpi_ids[] = {
> {"QCOM8001", QDF2XXX_V1},
> {"QCOM8002", QDF2XXX_V2},
> {},
> };
>
> struct hidma_cap {
> const struct of_device_id *of;
> const struct acpi_device_id *acpi;
> };
>
> static struct hidma_cap hidma_msi_cap = {
> hidma_msi_of_ids,
> hidma_msi_acpi_ids
> }
>
I think we are talking styles here. I started with your proposal and wanted to group
the settings together as much as I can for maintenance reasons only because
I don't have to remember that there are two different arrays that I need to take care
of when I add a new HW in the future.
+static struct hidma_cap hidma_msi_cap = {
+ .of = {
+ {.compatible = "qcom,hidma-1.1",},
+ {.compatible = "qcom,hidma-1.2",},
+ {},
+ },
+
+ .acpi = {
+ {"QCOM8062"},
+ {"QCOM8063"},
+ {},
+ }
+};
I like this better than what you are proposing.
> Keep in mind that you also need to add MODULE_DEVICE_TABLE entries, which you can't really do with your approach.
>
> MODULE_DEVICE_TABLE(of, hidma_msi_of_ids);
> MODULE_DEVICE_TABLE(acpi, hidma_msi_acpi_ids);
>
HIDMA capable devices are a subset of the devices that need to be probed. That's also
why I don't touch the device_table.
In the end both approaches work. It is a choice between what is more manageable. That
was the initial objection. I tried to close on this request.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 16:29 ` [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision Sinan Kaya
2017-11-08 16:49 ` Timur Tabi
@ 2017-11-08 17:51 ` Robin Murphy
2017-11-10 14:03 ` Sinan Kaya
1 sibling, 1 reply; 12+ messages in thread
From: Robin Murphy @ 2017-11-08 17:51 UTC (permalink / raw)
To: Sinan Kaya, dmaengine, timur
Cc: linux-arm-msm, linux-kernel, linux-arm-kernel
On 08/11/17 16:29, Sinan Kaya wrote:
> Add support for probing the newer HW and also organize MSI capable hardware
> into an array for maintenance reasons.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/dma/qcom/hidma.c | 41 +++++++++++++++++++++++++++++------------
> 1 file changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index e366985..4ef7d6f 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -50,6 +50,7 @@
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/of_dma.h>
> +#include <linux/of_device.h>
> #include <linux/property.h>
> #include <linux/delay.h>
> #include <linux/acpi.h>
> @@ -104,6 +105,26 @@ static void hidma_free(struct hidma_dev *dmadev)
> module_param(nr_desc_prm, uint, 0644);
> MODULE_PARM_DESC(nr_desc_prm, "number of descriptors (default: 0)");
>
> +#define HIDMA_MAX_DEV_MATCH 10
> +
> +struct hidma_cap {
> + const struct of_device_id of[HIDMA_MAX_DEV_MATCH];
> + const struct acpi_device_id acpi[HIDMA_MAX_DEV_MATCH];
> +};
> +
> +static struct hidma_cap hidma_msi_cap = {
> + .of = {
> + {.compatible = "qcom,hidma-1.1",},
> + {.compatible = "qcom,hidma-1.2",},
> + {},
> + },
> +
> + .acpi = {
> + {"QCOM8062"},
> + {"QCOM8063"},
> + {},
> + }
> +};
Yikes, I dread to imagine where this is going...
Apologies if I wasn't very clear, but what I meant to imply by dropping
the of_device_get_match_data() hint was to follow one of the common
patterns where you either just have some version token:
enum foo_ver {
FOO_V1,
...
}
struct acpi_device_id foo_acpi_ids[] = {
{ "_FOO0001", FOO_V1 },
...
}
struct of_device_id foo_of_match[] = {
{ .compatible = "foo,v1", .data = (void *)FOO_V1 },
...
}
int foo_probe(struct device *dev) {
...
foodev->version = (enum foo_ver)
of_device_get_match_data(&dev->of_node)
...
}
int foo_reset(struct foodev *foodev) {
if (foodev->version == FOO_V1)
writel(0, foodev->base + 0x20);
else
writel(0, foodev->base + 0x30);
}
or if it fits the code better, encapsulate the relevant details directly:
struct foodata {
.offset = 0x20,
} foo_v1_data;
struct acpi_device_id foo_acpi_ids[] = {
{ "_FOO0001", (unsigned long)&foo_v1_data },
...
}
struct of_device_id foo_of_match[] = {
{ .compatible = "foo,v1", .data = &foo_v1_data },
...
}
int foo_probe(struct device *dev) {
...
foodev->data = of_device_get_match_data(dev->of_node)
...
}
int foo_reset(struct foodev *foodev) {
writel(0, foodev->base + foodev->data->offset);
}
Creating multiple sets of match tables seems completely backwards, and
frankly looks worse than the open-coded strcmps IMO.
Robin.
>
> /* process completed descriptors */
> static void hidma_process_completed(struct hidma_chan *mchan)
> @@ -739,22 +760,16 @@ static int hidma_request_msi(struct hidma_dev *dmadev,
> static bool hidma_msi_capable(struct device *dev)
> {
> struct acpi_device *adev = ACPI_COMPANION(dev);
> - const char *of_compat;
> - int ret = -EINVAL;
> -
> - if (!adev || acpi_disabled) {
> - ret = device_property_read_string(dev, "compatible",
> - &of_compat);
> - if (ret)
> - return false;
> + int ret;
>
> - ret = strcmp(of_compat, "qcom,hidma-1.1");
> - } else {
> + if (!adev || acpi_disabled)
> + ret = of_match_device(hidma_msi_cap.of, dev) != NULL;
> + else {
> #ifdef CONFIG_ACPI
> - ret = strcmp(acpi_device_hid(adev), "QCOM8062");
> + ret = acpi_match_device(hidma_msi_cap.acpi, dev) != NULL;
> #endif
> }
> - return ret == 0;
> + return ret;
> }
>
> static int hidma_probe(struct platform_device *pdev)
> @@ -954,6 +969,7 @@ static int hidma_remove(struct platform_device *pdev)
> static const struct acpi_device_id hidma_acpi_ids[] = {
> {"QCOM8061"},
> {"QCOM8062"},
> + {"QCOM8063"},
> {},
> };
> MODULE_DEVICE_TABLE(acpi, hidma_acpi_ids);
> @@ -962,6 +978,7 @@ static int hidma_remove(struct platform_device *pdev)
> static const struct of_device_id hidma_match[] = {
> {.compatible = "qcom,hidma-1.0",},
> {.compatible = "qcom,hidma-1.1",},
> + {.compatible = "qcom,hidma-1.2",},
> {},
> };
> MODULE_DEVICE_TABLE(of, hidma_match);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 17:37 ` Sinan Kaya
@ 2017-11-08 17:51 ` Timur Tabi
0 siblings, 0 replies; 12+ messages in thread
From: Timur Tabi @ 2017-11-08 17:51 UTC (permalink / raw)
To: Sinan Kaya, dmaengine; +Cc: linux-arm-msm, linux-arm-kernel, linux-kernel
On 11/08/2017 11:37 AM, Sinan Kaya wrote:
> I think we are talking styles here.
I don't think my suggestions are stylistic. Your version wastes space.
However, if you really insist on your approach, that's fine with me.
I'm not the maintainer.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-08 17:51 ` Robin Murphy
@ 2017-11-10 14:03 ` Sinan Kaya
2017-11-10 14:14 ` Timur Tabi
2017-11-10 14:17 ` Robin Murphy
0 siblings, 2 replies; 12+ messages in thread
From: Sinan Kaya @ 2017-11-10 14:03 UTC (permalink / raw)
To: Robin Murphy, dmaengine, Timur Tabi
Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, linux-acpi,
Rafael J. Wysocki
+linux-acpi, +Rafael for context
On 11/8/2017 12:51 PM, Robin Murphy wrote:
> Apologies if I wasn't very clear, but what I meant to imply by dropping the of_device_get_match_data() hint was to follow one of the common patterns where you either just have some version token:
>
> enum foo_ver {
> FOO_V1,
> ...
> }
>
> struct acpi_device_id foo_acpi_ids[] = {
> { "_FOO0001", FOO_V1 },
> ...
> }
>
> struct of_device_id foo_of_match[] = {
> { .compatible = "foo,v1", .data = (void *)FOO_V1 },
> ...
> }
>
> int foo_probe(struct device *dev) {
> ...
> foodev->version = (enum foo_ver)
> of_device_get_match_data(&dev->of_node)
> ...
> }
>
> int foo_reset(struct foodev *foodev) {
> if (foodev->version == FOO_V1)
> writel(0, foodev->base + 0x20);
> else
> writel(0, foodev->base + 0x30);
> }
I did post v3 with this approach. However, I could not really find a ACPI function that
returns the driver data very similar to of_device_get_match_data(). The only thing
that is closer is acpi_match_device().
I introduced this new function as part of the v3 series.
Let me know if I'm missing something.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-10 14:03 ` Sinan Kaya
@ 2017-11-10 14:14 ` Timur Tabi
2017-11-10 14:17 ` Robin Murphy
1 sibling, 0 replies; 12+ messages in thread
From: Timur Tabi @ 2017-11-10 14:14 UTC (permalink / raw)
To: Sinan Kaya, Robin Murphy, dmaengine
Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, linux-acpi,
Rafael J. Wysocki
On 11/10/2017 08:03 AM, Sinan Kaya wrote:
> I did post v3 with this approach. However, I could not really find a ACPI function that
> returns the driver data very similar to of_device_get_match_data(). The only thing
> that is closer is acpi_match_device().
This is what I do in the EMAC driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c#n245
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
2017-11-10 14:03 ` Sinan Kaya
2017-11-10 14:14 ` Timur Tabi
@ 2017-11-10 14:17 ` Robin Murphy
1 sibling, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2017-11-10 14:17 UTC (permalink / raw)
To: Sinan Kaya, dmaengine, Timur Tabi
Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, linux-acpi,
Rafael J. Wysocki
On 10/11/17 14:03, Sinan Kaya wrote:
> +linux-acpi, +Rafael for context
>
> On 11/8/2017 12:51 PM, Robin Murphy wrote:
>> Apologies if I wasn't very clear, but what I meant to imply by dropping the of_device_get_match_data() hint was to follow one of the common patterns where you either just have some version token:
>>
>> enum foo_ver {
>> FOO_V1,
>> ...
>> }
>>
>> struct acpi_device_id foo_acpi_ids[] = {
>> { "_FOO0001", FOO_V1 },
>> ...
>> }
>>
>> struct of_device_id foo_of_match[] = {
>> { .compatible = "foo,v1", .data = (void *)FOO_V1 },
>> ...
>> }
>>
>> int foo_probe(struct device *dev) {
>> ...
>> foodev->version = (enum foo_ver)
>> of_device_get_match_data(&dev->of_node)
>> ...
>> }
>>
>> int foo_reset(struct foodev *foodev) {
>> if (foodev->version == FOO_V1)
>> writel(0, foodev->base + 0x20);
>> else
>> writel(0, foodev->base + 0x30);
>> }
>
> I did post v3 with this approach. However, I could not really find a ACPI function that
> returns the driver data very similar to of_device_get_match_data(). The only thing
> that is closer is acpi_match_device().
Yeah, I left the "follow the status quo and open-code it" part out of
the above example for brevity ;)
Probably 95% of the calls to acpi_match_device() are only doing so to
retrieve the driver_data, so the helper could provide scope for further
cleanup if anyone wants, too.
> I introduced this new function as part of the v3 series.
>
> Let me know if I'm missing something.
v3 looks good, thanks for persevering - I'll leave the rest up to Vinod
and Rafael.
Cheers,
Robin.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-11-10 14:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08 16:29 [PATCH V2 1/3] Documentation: DT: qcom_hidma: bump HW revision for the bugfixed HW Sinan Kaya
2017-11-08 16:29 ` [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision Sinan Kaya
2017-11-08 16:49 ` Timur Tabi
2017-11-08 16:58 ` Sinan Kaya
2017-11-08 17:12 ` Timur Tabi
2017-11-08 17:37 ` Sinan Kaya
2017-11-08 17:51 ` Timur Tabi
2017-11-08 17:51 ` Robin Murphy
2017-11-10 14:03 ` Sinan Kaya
2017-11-10 14:14 ` Timur Tabi
2017-11-10 14:17 ` Robin Murphy
2017-11-08 16:29 ` [PATCH V2 3/3] dmaengine: qcom_hidma: add identity register support 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).