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

[Sinan Kaya <okaya@codeaurora.org>]

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.