On 03/09/25 7:53 pm, Krzysztof Kozlowski wrote:
> On 03/09/2025 16:06, Andrew Lunn wrote:
>>>>> mboxes = <&mailbox0_cluster2 &mbox_main_r5fss0_core0>;
>>>>> memory-region = <&main_r5fss0_core0_dma_memory_region>,
>>>>> <&main_r5fss0_core0_memory_region>;
>>>>> + rpmsg-eth-region = <&main_r5fss0_core0_memory_region_shm>;
>>>>
>>>> You already have here memory-region, so use that one.
>>>>
>>>
>>> There is a problem with using memory-region. If I add
>>> `main_r5fss0_core0_memory_region_shm` to memory region, to get this
>>> phandle from driver I would have to use
>>>
>>> of_parse_phandle(np, "memory-region", 2)
>>>
>>> Where 2 is the index for this region. But the problem is how would the
>>> driver know this index. This index can vary for different vendors and
>>> their rproc device.
>>>
>>> If some other vendor tries to use this driver but their memory-region
>>> has 3 existing entries. so this this entry will be the 4th one.
>>
>> Just adding to this, there is nothing really TI specific in this
>> system. We want the design so that any vendor can use it, just by
>> adding the needed nodes to their rpmsg node, indicating there is a
>> compatible implementation on the other end, and an indication of where
>> the shared memory is.
>
> I don't know your drivers, but I still do not see here a problem with
> 'memory-region'. You just need to tell this common code which
> memory-region phandle by index or name is the one for rpmsg.
>
I am able to pass this index as driver_data in the `rpmsg_device_id`.
I can work with just adding this reserved memory region to the
'memory-region'. No need to create additional node in dt.
This will be the code,
static const struct rpmsg_eth_data ti_rpmsg_eth_data = {
.shm_region_index = 2,
};
static struct rpmsg_device_id rpmsg_eth_rpmsg_id_table[] = {
{ .name = "ti.shm-eth", .driver_data =
(kernel_ulong_t)&ti_rpmsg_eth_data },
{},
};
Other vendors can add separate entry to rpmsg_eth_rpmsg_id_table with
their index based on their device tree.
I am keeping the data name `ti_rpmsg_eth_data` vendor specific so that
other vendors can create their data filed and add entry to
`rpmsg_eth_rpmsg_id_table` with driver_data pointing to their data
structure.
Andrew, does this sound ok to you. I will send out a v3 once you confirm.
Krzysztof, Andrew thanks for the feedback.
--
Thanks and Regards,
Danish
