Re: [PATCH V6 4/7] OF: properties: Implement get_match_data() callback

From: Sinan Kaya
Date: Fri Dec 08 2017 - 09:40:16 EST


On 12/8/2017 9:33 AM, Sinan Kaya wrote:
> On 12/8/2017 4:11 AM, Lothar WaÃmann wrote:
>>> While implementing the ACPI piece, I have to convert an unsigned long to
>>> (const void *) in ACPI code so that the APIs are compatible.
>>>
>> Just one more remark: Do you need write access to the data the pointer
>> returned by device_get_match_data() or of_fwnode_get_match_data()
>> points to?
>> If not, the return type of those functions should be 'const void *'.
>
> Yes, the only reason driver is trying to obtain this data pointer is to modify
> members of it. I did a quick test with your suggestion.

I guess I should soften my statement here. I look at examples, they seem to be read.
Even for my HIDMA case:

static const struct of_device_id hidma_match[] = {
{.compatible = "qcom,hidma-1.0",},
{.compatible = "qcom,hidma-1.1", .data = (void *)(HIDMA_MSI_CAP),},
{.compatible = "qcom,hidma-1.2",
.data = (void *)(HIDMA_MSI_CAP | HIDMA_IDENTITY_CAP),},
{},
};

I can change the return type to (const void*) if Rob, Rafael and Sakari agrees.
I didn't really like converting a long to const void*. That's my personal opinion.

>
> test.c: In function âmainâ:
> test.c:15:2: error: assignment of member âmâ in read-only object
> t->m = 3;
> ^
>
> struct test
> {
> int m;
> };
>
> int main(void)
> {
> const void *ptr;
> unsigned long l =4;
> const struct test *t;
>
> ptr = (const void *)l;
>
> t = ptr;
> t->m = 3;
> }
>
>
>


--
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.