On 01/09/2017 05:45 PM, Bjorn Andersson wrote:
> On Fri 30 Dec 11:56 PST 2016, Bjorn Andersson wrote:
>
>> On Tue 27 Dec 14:36 PST 2016, Suman Anna wrote:
>>
>>> Hi Loic,
>>>
>>> On 12/15/2016 11:09 PM, Bjorn Andersson wrote:
>>>> On Thu 15 Dec 06:49 PST 2016, Loic Pallardy wrote:
>>>>
>>>>> Since commit 4dffed5b3ac796b ("rpmsg: Name rpmsg devices based on
>>>>> channel id"), it is no more possible for a firmware to register twice
>>>>> a service (on different endpoints). rpmsg_register_device function
>>>>> is failing when calling device_add for the second time as second
>>>>> device has the same name as first one already register.
>>>>> It is because name is based only on service name and so is not more
>>>>> unique. Previously name was unique thanks to the use of rpmsg_dev_index.
>>>>>
>>>>> This patch adds destination and source endpoint numbers device name to
>>>>> create an unique identifier.
>>>>>
>>>>> Signed-off-by: Loic Pallardy <loic.pallardy@xxxxxx>
>>>>
>>>> Looks good, thanks.
>>>
>>> Thanks for the patch, I have ran into the exact same issue as well.
>>>
>>>>
>>>> Regards,
>>>> Bjorn
>>>>
>>>>> ---
>>>>> v2: Update commit header with commit ID generating regression
>>>>> Fix rpmsg_core instead of virtio_rpmsg
>>>>>
>>>>> drivers/rpmsg/rpmsg_core.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
>>>>> index a79cb5a..18c73e0 100644
>>>>> --- a/drivers/rpmsg/rpmsg_core.c
>>>>> +++ b/drivers/rpmsg/rpmsg_core.c
>>>>> @@ -453,8 +453,8 @@ int rpmsg_register_device(struct rpmsg_device *rpdev)
>>>>> struct device *dev = &rpdev->dev;
>>>>> int ret;
>>>>>
>>>>> - dev_set_name(&rpdev->dev, "%s:%s",
>>>>> - dev_name(dev->parent), rpdev->id.name);
>>>>> + dev_set_name(&rpdev->dev, "%s.%d.%d.%s", dev_name(dev->parent),
>>>>> + rpdev->src, rpdev->dst, rpdev->id.name);
>>>
>>> This is fine, though I would have preferred the numbers at the end after
>>> the string literals (%s.%s.%d.%d instead of %s.%d.%d.%s)
>>>
>>
>> When using the Qualcomm SMD backend only the name is used to identify
>> channels, so I set src & dst to RPMSG_ADDR_ANY. Moving the numbers to
>> the end makes this slightly better looking and one could potentially
>> trim those without changing the structure of the name.
>>
>>
>> Loic, I took the liberty of changing the order and have updated my
>> for-next [1] with the following patches:
>>
>> a0c10687ec95 Revert "remoteproc: Merge table_ptr and cached_table pointers"
>> c81c0e0710f0 remoteproc: fix vdev reference management
>> 63447646ac65 rpmsg: virtio_rpmsg_bus: fix channel creation
>>
>>
>> I will let them sit there a few days to get some additional build and
>> boot test and hope you can give them a spin as well; before I send them
>> to Linus.
>>
>> [1] https://github.com/andersson/remoteproc/commits/for-next
>>
>
> Loic, did you have a chance to give this a test run? I would appreciate
> your feedback before I send this off to Linus.
>
> (The fixed version is part of linux-next as well)
FWIW, these work fine on TI platforms. I had run into the same issues
that these commits were fixing.
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
