Re: [PATCH 2/2] tty: add rpmsg driver
On Mon, Apr 15, 2019 at 9:14 PM Enrico Weigelt, metux IT consult wrote: > > On 12.04.19 18:00, Arnaud Pouliquen wrote: > > Hi folks, > > > > Haven't followed the whole thread, but I've got the impression that the > device is emulating an uart - if that's the case wouldn't it be better > to implement a serial driver, instead of tty directly (which IMHO should > make it also usable for serbus-based devices) ? It's more natural to implement rpmsg uart on top of tty core instead of serial layer, since: 1.Serial layer model the hardware which can just send the data one by one 2.But rpmsg could send a buffer(max 512B) in one packet On the other hand, it's very easy to support serbus by replace tty_port_register_device_attr to tty_port_register_device_attr_serdev. > > > --mtx > > -- > Enrico Weigelt, metux IT consult > Free software and Linux embedded engineering > i...@metux.net -- +49-151-27565287
Re: [PATCH 2/2] tty: add rpmsg driver
On 12.04.19 18:00, Arnaud Pouliquen wrote: Hi folks, Haven't followed the whole thread, but I've got the impression that the device is emulating an uart - if that's the case wouldn't it be better to implement a serial driver, instead of tty directly (which IMHO should make it also usable for serbus-based devices) ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
Re: [PATCH 2/2] tty: add rpmsg driver
On 4/9/19 12:14 PM, xiang xiao wrote:
> On Tue, Apr 9, 2019 at 3:28 PM Arnaud Pouliquen
> wrote:
>>
>>
>>
>> On 4/8/19 3:29 PM, xiang xiao wrote:
>>> On Mon, Apr 8, 2019 at 8:05 PM Arnaud Pouliquen
>>> wrote:
On 4/6/19 9:56 AM, xiang xiao wrote:
> On Sat, Apr 6, 2019 at 12:08 AM Arnaud Pouliquen
> wrote:
>>
>>
>>
>> On 4/5/19 4:03 PM, xiang xiao wrote:
>>> On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
>>> wrote:
On 4/5/19 12:12 PM, xiang xiao wrote:
> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> wrote:
>>
>> Hello Xiang,
>>
>>
>> On 4/3/19 2:47 PM, xiang xiao wrote:
>>> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
>>> wrote:
This driver exposes a standard tty interface on top of the rpmsg
framework through the "rpmsg-tty-channel" rpmsg service.
This driver supports multi-instances, offering a /dev/ttyRPMSGx
entry
per rpmsg endpoint.
>>>
>>> How to support multi-instances from the same remoteproc instance? I
>>> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
>>> only one channel can be created for each remote side.
>> The driver is multi-instance based on muti-endpoints on top of the
>> "rpmsg-tty-channel" service.
>> On remote side you just have to call rpmsg_create_ept with
>> destination
>> address set to ANY. The result is a NS service announcement that
>> trigs a
>> probe with a new endpoint.
>> You can find code example for the remote side here:
>> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
>
> Demo code create two uarts(huart0 and huart1), and both use the same
> channel name( "rpmsg-tty-channel").
> But rpmsg_create_channel in kernel will complain the duplication:
> /* make sure a similar channel doesn't already exist */
> tmp = rpmsg_find_device(dev, chinfo);
> if (tmp) {
> /* decrement the matched device's refcount back */
> put_device(tmp);
> dev_err(dev, "channel %s:%x:%x already exist\n",
>chinfo->name, chinfo->src, chinfo->dst);
> return NULL;
> }
> Do you have some local change not upstream yet?
Nothing is missing. There is no complain as the function
rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
to the remote ept address) is different.
>>>
>>> Yes, you are right.
>>>
If i well remember you have also a similar implementation of the
service
on your side. Do you see any incompatibility with your implementation?
>>>
>>> Our implementation is similar to yours, but has two major difference:
>>> 1.Each instance has a different channel name but share the same prefix
>>> "rpmsg-tty*", the benefit is that:
>>>a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
>>> random /dev/ttyRPMSGx
>>>b.Don't need tty_idr to allocate the unique device id
>> I understand the need but in your implementation it look like you hack
>> the rpmsg service to instantiate your tty... you introduce a kind of
>> meta rpmsg tty service with sub-service related to the serial content.
>> Not sure that this could be upstreamed...
>
> Not too much hack here, the only change in common is:
> 1.Add match callback into rpmsg_driver
> 2.Call match callback in rpmsg_dev_match
> so rpmsg driver could join the bus match decision process(e.g. change
> the exact match to the prefix match).
> The similar mechanism exist in other subsystem for many years.
The mechanism also exists in rpmsg but based on the service. it is
similar to the compatible, based on the rpmsg_device_id structure that
should list the cervices supported.
>>>
>>> But match callback is much flexible than rpmsg_device_id table, the
>>> table is fixed at compile time, match callback could do all matic at
>>> the runtime.
>> Today this not the way rpmsg implements the service but declares it on
>> registration. This is an evolution of the rpmsg, so better to propose it
>> in a separate thread.
>>
>
> Here is the patch I post before:
> https://patchwork.kernel.org/patch/10791741/
>
>>>
My concern here is that you would like to expose the service on top of
the tty while aim of this driver is just to expose a tty over rpmsg. So
in this case seems not a generic implementation but a platform dependent
implementation.
>>>
>>> I can't understand why the implementation is platfo
Re: [PATCH 2/2] tty: add rpmsg driver
On Tue, Apr 9, 2019 at 3:28 PM Arnaud Pouliquen wrote:
>
>
>
> On 4/8/19 3:29 PM, xiang xiao wrote:
> > On Mon, Apr 8, 2019 at 8:05 PM Arnaud Pouliquen
> > wrote:
> >>
> >>
> >>
> >> On 4/6/19 9:56 AM, xiang xiao wrote:
> >>> On Sat, Apr 6, 2019 at 12:08 AM Arnaud Pouliquen
> >>> wrote:
>
>
>
> On 4/5/19 4:03 PM, xiang xiao wrote:
> > On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
> > wrote:
> >>
> >>
> >>
> >> On 4/5/19 12:12 PM, xiang xiao wrote:
> >>> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> >>> wrote:
>
> Hello Xiang,
>
>
> On 4/3/19 2:47 PM, xiang xiao wrote:
> > On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> > wrote:
> >>
> >> This driver exposes a standard tty interface on top of the rpmsg
> >> framework through the "rpmsg-tty-channel" rpmsg service.
> >>
> >> This driver supports multi-instances, offering a /dev/ttyRPMSGx
> >> entry
> >> per rpmsg endpoint.
> >>
> >
> > How to support multi-instances from the same remoteproc instance? I
> > saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> > only one channel can be created for each remote side.
> The driver is multi-instance based on muti-endpoints on top of the
> "rpmsg-tty-channel" service.
> On remote side you just have to call rpmsg_create_ept with
> destination
> address set to ANY. The result is a NS service announcement that
> trigs a
> probe with a new endpoint.
> You can find code example for the remote side here:
> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
> >>>
> >>> Demo code create two uarts(huart0 and huart1), and both use the same
> >>> channel name( "rpmsg-tty-channel").
> >>> But rpmsg_create_channel in kernel will complain the duplication:
> >>> /* make sure a similar channel doesn't already exist */
> >>> tmp = rpmsg_find_device(dev, chinfo);
> >>> if (tmp) {
> >>> /* decrement the matched device's refcount back */
> >>> put_device(tmp);
> >>> dev_err(dev, "channel %s:%x:%x already exist\n",
> >>>chinfo->name, chinfo->src, chinfo->dst);
> >>> return NULL;
> >>> }
> >>> Do you have some local change not upstream yet?
> >> Nothing is missing. There is no complain as the function
> >> rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
> >> to the remote ept address) is different.
> >>
> >
> > Yes, you are right.
> >
> >> If i well remember you have also a similar implementation of the
> >> service
> >> on your side. Do you see any incompatibility with your implementation?
> >>
> >
> > Our implementation is similar to yours, but has two major difference:
> > 1.Each instance has a different channel name but share the same prefix
> > "rpmsg-tty*", the benefit is that:
> >a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
> > random /dev/ttyRPMSGx
> >b.Don't need tty_idr to allocate the unique device id
> I understand the need but in your implementation it look like you hack
> the rpmsg service to instantiate your tty... you introduce a kind of
> meta rpmsg tty service with sub-service related to the serial content.
> Not sure that this could be upstreamed...
> >>>
> >>> Not too much hack here, the only change in common is:
> >>> 1.Add match callback into rpmsg_driver
> >>> 2.Call match callback in rpmsg_dev_match
> >>> so rpmsg driver could join the bus match decision process(e.g. change
> >>> the exact match to the prefix match).
> >>> The similar mechanism exist in other subsystem for many years.
> >> The mechanism also exists in rpmsg but based on the service. it is
> >> similar to the compatible, based on the rpmsg_device_id structure that
> >> should list the cervices supported.
> >
> > But match callback is much flexible than rpmsg_device_id table, the
> > table is fixed at compile time, match callback could do all matic at
> > the runtime.
> Today this not the way rpmsg implements the service but declares it on
> registration. This is an evolution of the rpmsg, so better to propose it
> in a separate thread.
>
Here is the patch I post before:
https://patchwork.kernel.org/patch/10791741/
> >
> >> My concern here is that you would like to expose the service on top of
> >> the tty while aim of this driver is just to expose a tty over rpmsg. So
> >> in this case seems not a generic implementation but a platform dependent
> >> implementation.
> >>
> >
> > I can't understand why the implementation is platform dependent, could
> > you explain more details?In your uart_
Re: [PATCH 2/2] tty: add rpmsg driver
On 4/8/19 3:29 PM, xiang xiao wrote:
> On Mon, Apr 8, 2019 at 8:05 PM Arnaud Pouliquen
> wrote:
>>
>>
>>
>> On 4/6/19 9:56 AM, xiang xiao wrote:
>>> On Sat, Apr 6, 2019 at 12:08 AM Arnaud Pouliquen
>>> wrote:
On 4/5/19 4:03 PM, xiang xiao wrote:
> On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
> wrote:
>>
>>
>>
>> On 4/5/19 12:12 PM, xiang xiao wrote:
>>> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
>>> wrote:
Hello Xiang,
On 4/3/19 2:47 PM, xiang xiao wrote:
> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> wrote:
>>
>> This driver exposes a standard tty interface on top of the rpmsg
>> framework through the "rpmsg-tty-channel" rpmsg service.
>>
>> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
>> per rpmsg endpoint.
>>
>
> How to support multi-instances from the same remoteproc instance? I
> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> only one channel can be created for each remote side.
The driver is multi-instance based on muti-endpoints on top of the
"rpmsg-tty-channel" service.
On remote side you just have to call rpmsg_create_ept with destination
address set to ANY. The result is a NS service announcement that trigs
a
probe with a new endpoint.
You can find code example for the remote side here:
https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
>>>
>>> Demo code create two uarts(huart0 and huart1), and both use the same
>>> channel name( "rpmsg-tty-channel").
>>> But rpmsg_create_channel in kernel will complain the duplication:
>>> /* make sure a similar channel doesn't already exist */
>>> tmp = rpmsg_find_device(dev, chinfo);
>>> if (tmp) {
>>> /* decrement the matched device's refcount back */
>>> put_device(tmp);
>>> dev_err(dev, "channel %s:%x:%x already exist\n",
>>>chinfo->name, chinfo->src, chinfo->dst);
>>> return NULL;
>>> }
>>> Do you have some local change not upstream yet?
>> Nothing is missing. There is no complain as the function
>> rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
>> to the remote ept address) is different.
>>
>
> Yes, you are right.
>
>> If i well remember you have also a similar implementation of the service
>> on your side. Do you see any incompatibility with your implementation?
>>
>
> Our implementation is similar to yours, but has two major difference:
> 1.Each instance has a different channel name but share the same prefix
> "rpmsg-tty*", the benefit is that:
>a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
> random /dev/ttyRPMSGx
>b.Don't need tty_idr to allocate the unique device id
I understand the need but in your implementation it look like you hack
the rpmsg service to instantiate your tty... you introduce a kind of
meta rpmsg tty service with sub-service related to the serial content.
Not sure that this could be upstreamed...
>>>
>>> Not too much hack here, the only change in common is:
>>> 1.Add match callback into rpmsg_driver
>>> 2.Call match callback in rpmsg_dev_match
>>> so rpmsg driver could join the bus match decision process(e.g. change
>>> the exact match to the prefix match).
>>> The similar mechanism exist in other subsystem for many years.
>> The mechanism also exists in rpmsg but based on the service. it is
>> similar to the compatible, based on the rpmsg_device_id structure that
>> should list the cervices supported.
>
> But match callback is much flexible than rpmsg_device_id table, the
> table is fixed at compile time, match callback could do all matic at
> the runtime.
Today this not the way rpmsg implements the service but declares it on
registration. This is an evolution of the rpmsg, so better to propose it
in a separate thread.
>
>> My concern here is that you would like to expose the service on top of
>> the tty while aim of this driver is just to expose a tty over rpmsg. So
>> in this case seems not a generic implementation but a platform dependent
>> implementation.
>>
>
> I can't understand why the implementation is platform dependent, could
> you explain more details?In your uart_rpmsg/c.
the rpmsg service is "rpmsg-tty" this is a "standard" service. But you
define a "rpmsg-tty" service because you want to expose a service on
top of the tty service, not the tty service itself. In this way you are
not able to list this service in rpmsg_device_id because not standard
static service, you have to implement the match. This look like you
adapt rpmsg protocol to ma
Re: [PATCH 2/2] tty: add rpmsg driver
On Mon, Apr 8, 2019 at 8:05 PM Arnaud Pouliquen wrote:
>
>
>
> On 4/6/19 9:56 AM, xiang xiao wrote:
> > On Sat, Apr 6, 2019 at 12:08 AM Arnaud Pouliquen
> > wrote:
> >>
> >>
> >>
> >> On 4/5/19 4:03 PM, xiang xiao wrote:
> >>> On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
> >>> wrote:
>
>
>
> On 4/5/19 12:12 PM, xiang xiao wrote:
> > On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> > wrote:
> >>
> >> Hello Xiang,
> >>
> >>
> >> On 4/3/19 2:47 PM, xiang xiao wrote:
> >>> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> >>> wrote:
>
> This driver exposes a standard tty interface on top of the rpmsg
> framework through the "rpmsg-tty-channel" rpmsg service.
>
> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> per rpmsg endpoint.
>
> >>>
> >>> How to support multi-instances from the same remoteproc instance? I
> >>> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> >>> only one channel can be created for each remote side.
> >> The driver is multi-instance based on muti-endpoints on top of the
> >> "rpmsg-tty-channel" service.
> >> On remote side you just have to call rpmsg_create_ept with destination
> >> address set to ANY. The result is a NS service announcement that trigs
> >> a
> >> probe with a new endpoint.
> >> You can find code example for the remote side here:
> >> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
> >
> > Demo code create two uarts(huart0 and huart1), and both use the same
> > channel name( "rpmsg-tty-channel").
> > But rpmsg_create_channel in kernel will complain the duplication:
> > /* make sure a similar channel doesn't already exist */
> > tmp = rpmsg_find_device(dev, chinfo);
> > if (tmp) {
> > /* decrement the matched device's refcount back */
> > put_device(tmp);
> > dev_err(dev, "channel %s:%x:%x already exist\n",
> >chinfo->name, chinfo->src, chinfo->dst);
> > return NULL;
> > }
> > Do you have some local change not upstream yet?
> Nothing is missing. There is no complain as the function
> rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
> to the remote ept address) is different.
>
> >>>
> >>> Yes, you are right.
> >>>
> If i well remember you have also a similar implementation of the service
> on your side. Do you see any incompatibility with your implementation?
>
> >>>
> >>> Our implementation is similar to yours, but has two major difference:
> >>> 1.Each instance has a different channel name but share the same prefix
> >>> "rpmsg-tty*", the benefit is that:
> >>>a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
> >>> random /dev/ttyRPMSGx
> >>>b.Don't need tty_idr to allocate the unique device id
> >> I understand the need but in your implementation it look like you hack
> >> the rpmsg service to instantiate your tty... you introduce a kind of
> >> meta rpmsg tty service with sub-service related to the serial content.
> >> Not sure that this could be upstreamed...
> >
> > Not too much hack here, the only change in common is:
> > 1.Add match callback into rpmsg_driver
> > 2.Call match callback in rpmsg_dev_match
> > so rpmsg driver could join the bus match decision process(e.g. change
> > the exact match to the prefix match).
> > The similar mechanism exist in other subsystem for many years.
> The mechanism also exists in rpmsg but based on the service. it is
> similar to the compatible, based on the rpmsg_device_id structure that
> should list the cervices supported.
But match callback is much flexible than rpmsg_device_id table, the
table is fixed at compile time, match callback could do all matic at
the runtime.
> My concern here is that you would like to expose the service on top of
> the tty while aim of this driver is just to expose a tty over rpmsg. So
> in this case seems not a generic implementation but a platform dependent
> implementation.
>
I can't understand why the implementation is platform dependent, could
you explain more details?
> >
> >> proposal to integrate your need in the ST driver: it seems possible to
> >> have /dev/ttyRPMSGx with x corresponding to the remote endpoint address?
> >> So if you want to have a fixed tty name you can fix the remote endpoint.
> >> Is it something reasonable for you?
> >
> > But in our system, we have more than ten rpmsg services running at the
> > same time, it's difficult to manage them by the hardcode endpoint
> > address.
> Seems not so difficult. Today you identify your service by a name. Seems
> just a matter of changing it by an address, it just an identifier by an
> address instead of a string.
But I still prefer t
Re: [PATCH 2/2] tty: add rpmsg driver
On 4/6/19 9:56 AM, xiang xiao wrote:
> On Sat, Apr 6, 2019 at 12:08 AM Arnaud Pouliquen
> wrote:
>>
>>
>>
>> On 4/5/19 4:03 PM, xiang xiao wrote:
>>> On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
>>> wrote:
On 4/5/19 12:12 PM, xiang xiao wrote:
> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> wrote:
>>
>> Hello Xiang,
>>
>>
>> On 4/3/19 2:47 PM, xiang xiao wrote:
>>> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
>>> wrote:
This driver exposes a standard tty interface on top of the rpmsg
framework through the "rpmsg-tty-channel" rpmsg service.
This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
per rpmsg endpoint.
>>>
>>> How to support multi-instances from the same remoteproc instance? I
>>> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
>>> only one channel can be created for each remote side.
>> The driver is multi-instance based on muti-endpoints on top of the
>> "rpmsg-tty-channel" service.
>> On remote side you just have to call rpmsg_create_ept with destination
>> address set to ANY. The result is a NS service announcement that trigs a
>> probe with a new endpoint.
>> You can find code example for the remote side here:
>> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
>
> Demo code create two uarts(huart0 and huart1), and both use the same
> channel name( "rpmsg-tty-channel").
> But rpmsg_create_channel in kernel will complain the duplication:
> /* make sure a similar channel doesn't already exist */
> tmp = rpmsg_find_device(dev, chinfo);
> if (tmp) {
> /* decrement the matched device's refcount back */
> put_device(tmp);
> dev_err(dev, "channel %s:%x:%x already exist\n",
>chinfo->name, chinfo->src, chinfo->dst);
> return NULL;
> }
> Do you have some local change not upstream yet?
Nothing is missing. There is no complain as the function
rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
to the remote ept address) is different.
>>>
>>> Yes, you are right.
>>>
If i well remember you have also a similar implementation of the service
on your side. Do you see any incompatibility with your implementation?
>>>
>>> Our implementation is similar to yours, but has two major difference:
>>> 1.Each instance has a different channel name but share the same prefix
>>> "rpmsg-tty*", the benefit is that:
>>>a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
>>> random /dev/ttyRPMSGx
>>>b.Don't need tty_idr to allocate the unique device id
>> I understand the need but in your implementation it look like you hack
>> the rpmsg service to instantiate your tty... you introduce a kind of
>> meta rpmsg tty service with sub-service related to the serial content.
>> Not sure that this could be upstreamed...
>
> Not too much hack here, the only change in common is:
> 1.Add match callback into rpmsg_driver
> 2.Call match callback in rpmsg_dev_match
> so rpmsg driver could join the bus match decision process(e.g. change
> the exact match to the prefix match).
> The similar mechanism exist in other subsystem for many years.
The mechanism also exists in rpmsg but based on the service. it is
similar to the compatible, based on the rpmsg_device_id structure that
should list the cervices supported.
My concern here is that you would like to expose the service on top of
the tty while aim of this driver is just to expose a tty over rpmsg. So
in this case seems not a generic implementation but a platform dependent
implementation.
>
>> proposal to integrate your need in the ST driver: it seems possible to
>> have /dev/ttyRPMSGx with x corresponding to the remote endpoint address?
>> So if you want to have a fixed tty name you can fix the remote endpoint.
>> Is it something reasonable for you?
>
> But in our system, we have more than ten rpmsg services running at the
> same time, it's difficult to manage them by the hardcode endpoint
> address.
Seems not so difficult. Today you identify your service by a name. Seems
just a matter of changing it by an address, it just an identifier by an
address instead of a string.
>
>>
>>
>>> 2.Each transfer need get response from peer to avoid the buffer
>>> overflow. This is very important if the peer use pull
>>> model(read/write) instead of push model(callback).
>> I not sure to understand your point... You mean that you assume that the
>> driver should be blocked until a response from the remote side?
>
> For example, in your RTOS demo code:
> 1.VIRT_UART0_RxCpltCallback save the received data in a global buffer
> VirtUart0ChannelBuffRx
> 2.Main loop poll VirtUart0RxMsg flag and echo the data back to ker
Re: [PATCH 2/2] tty: add rpmsg driver
On Sat, Apr 6, 2019 at 12:08 AM Arnaud Pouliquen
wrote:
>
>
>
> On 4/5/19 4:03 PM, xiang xiao wrote:
> > On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
> > wrote:
> >>
> >>
> >>
> >> On 4/5/19 12:12 PM, xiang xiao wrote:
> >>> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> >>> wrote:
>
> Hello Xiang,
>
>
> On 4/3/19 2:47 PM, xiang xiao wrote:
> > On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> > wrote:
> >>
> >> This driver exposes a standard tty interface on top of the rpmsg
> >> framework through the "rpmsg-tty-channel" rpmsg service.
> >>
> >> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> >> per rpmsg endpoint.
> >>
> >
> > How to support multi-instances from the same remoteproc instance? I
> > saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> > only one channel can be created for each remote side.
> The driver is multi-instance based on muti-endpoints on top of the
> "rpmsg-tty-channel" service.
> On remote side you just have to call rpmsg_create_ept with destination
> address set to ANY. The result is a NS service announcement that trigs a
> probe with a new endpoint.
> You can find code example for the remote side here:
> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
> >>>
> >>> Demo code create two uarts(huart0 and huart1), and both use the same
> >>> channel name( "rpmsg-tty-channel").
> >>> But rpmsg_create_channel in kernel will complain the duplication:
> >>> /* make sure a similar channel doesn't already exist */
> >>> tmp = rpmsg_find_device(dev, chinfo);
> >>> if (tmp) {
> >>> /* decrement the matched device's refcount back */
> >>> put_device(tmp);
> >>> dev_err(dev, "channel %s:%x:%x already exist\n",
> >>>chinfo->name, chinfo->src, chinfo->dst);
> >>> return NULL;
> >>> }
> >>> Do you have some local change not upstream yet?
> >> Nothing is missing. There is no complain as the function
> >> rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
> >> to the remote ept address) is different.
> >>
> >
> > Yes, you are right.
> >
> >> If i well remember you have also a similar implementation of the service
> >> on your side. Do you see any incompatibility with your implementation?
> >>
> >
> > Our implementation is similar to yours, but has two major difference:
> > 1.Each instance has a different channel name but share the same prefix
> > "rpmsg-tty*", the benefit is that:
> >a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
> > random /dev/ttyRPMSGx
> >b.Don't need tty_idr to allocate the unique device id
> I understand the need but in your implementation it look like you hack
> the rpmsg service to instantiate your tty... you introduce a kind of
> meta rpmsg tty service with sub-service related to the serial content.
> Not sure that this could be upstreamed...
Not too much hack here, the only change in common is:
1.Add match callback into rpmsg_driver
2.Call match callback in rpmsg_dev_match
so rpmsg driver could join the bus match decision process(e.g. change
the exact match to the prefix match).
The similar mechanism exist in other subsystem for many years.
> proposal to integrate your need in the ST driver: it seems possible to
> have /dev/ttyRPMSGx with x corresponding to the remote endpoint address?
> So if you want to have a fixed tty name you can fix the remote endpoint.
> Is it something reasonable for you?
But in our system, we have more than ten rpmsg services running at the
same time, it's difficult to manage them by the hardcode endpoint
address.
>
>
> > 2.Each transfer need get response from peer to avoid the buffer
> > overflow. This is very important if the peer use pull
> > model(read/write) instead of push model(callback).
> I not sure to understand your point... You mean that you assume that the
> driver should be blocked until a response from the remote side?
For example, in your RTOS demo code:
1.VIRT_UART0_RxCpltCallback save the received data in a global buffer
VirtUart0ChannelBuffRx
2.Main loop poll VirtUart0RxMsg flag and echo the data back to kernel
if this flag is set
Between step1 and step 2, kernel may send additional data and then
overwrite the data not get processed by main loop.
It's very easy to reproduce by:
cat /dev/ttyRPMSGx > /tmp/dump &
cat /a/huge/file > /dev/ttyRPMSGx
diff /a/hug/file /tmp/dump
The push model mean the receiver could process the data completely in
callback context, and
the pull model mean the receiver just save the data in buffer and
process it late(e.g. by read call).
> This seems not compatible with a "generic" tty and with Johan remarks:
> "Just a drive-by comment; it looks like rpmsg_send() may block, but
> the tty-driver write() callback must never sleep."
>
The handsh
Re: [PATCH 2/2] tty: add rpmsg driver
On 4/5/19 4:03 PM, xiang xiao wrote:
> On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen
> wrote:
>>
>>
>>
>> On 4/5/19 12:12 PM, xiang xiao wrote:
>>> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
>>> wrote:
Hello Xiang,
On 4/3/19 2:47 PM, xiang xiao wrote:
> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> wrote:
>>
>> This driver exposes a standard tty interface on top of the rpmsg
>> framework through the "rpmsg-tty-channel" rpmsg service.
>>
>> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
>> per rpmsg endpoint.
>>
>
> How to support multi-instances from the same remoteproc instance? I
> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> only one channel can be created for each remote side.
The driver is multi-instance based on muti-endpoints on top of the
"rpmsg-tty-channel" service.
On remote side you just have to call rpmsg_create_ept with destination
address set to ANY. The result is a NS service announcement that trigs a
probe with a new endpoint.
You can find code example for the remote side here:
https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
>>>
>>> Demo code create two uarts(huart0 and huart1), and both use the same
>>> channel name( "rpmsg-tty-channel").
>>> But rpmsg_create_channel in kernel will complain the duplication:
>>> /* make sure a similar channel doesn't already exist */
>>> tmp = rpmsg_find_device(dev, chinfo);
>>> if (tmp) {
>>> /* decrement the matched device's refcount back */
>>> put_device(tmp);
>>> dev_err(dev, "channel %s:%x:%x already exist\n",
>>>chinfo->name, chinfo->src, chinfo->dst);
>>> return NULL;
>>> }
>>> Do you have some local change not upstream yet?
>> Nothing is missing. There is no complain as the function
>> rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
>> to the remote ept address) is different.
>>
>
> Yes, you are right.
>
>> If i well remember you have also a similar implementation of the service
>> on your side. Do you see any incompatibility with your implementation?
>>
>
> Our implementation is similar to yours, but has two major difference:
> 1.Each instance has a different channel name but share the same prefix
> "rpmsg-tty*", the benefit is that:
>a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
> random /dev/ttyRPMSGx
>b.Don't need tty_idr to allocate the unique device id
I understand the need but in your implementation it look like you hack
the rpmsg service to instantiate your tty... you introduce a kind of
meta rpmsg tty service with sub-service related to the serial content.
Not sure that this could be upstreamed...
proposal to integrate your need in the ST driver: it seems possible to
have /dev/ttyRPMSGx with x corresponding to the remote endpoint address?
So if you want to have a fixed tty name you can fix the remote endpoint.
Is it something reasonable for you?
> 2.Each transfer need get response from peer to avoid the buffer
> overflow. This is very important if the peer use pull
> model(read/write) instead of push model(callback).
I not sure to understand your point... You mean that you assume that the
driver should be blocked until a response from the remote side?
This seems not compatible with a "generic" tty and with Johan remarks:
"Just a drive-by comment; it looks like rpmsg_send() may block, but
the tty-driver write() callback must never sleep."
Why no just let rpmsg should manage the case with no Tx buffer free,
with a rpmsg_trysend...
>
> Here is the patch for kernel side:
> https://github.com/xiaoxiang781216/linux/commit/f04d2386eb11e0781f0eb47d99fae838abf7eb53
> https://github.com/xiaoxiang781216/linux/commit/1a41be362d916eb9104bf21065cb38a0a63d2e91
>
> And RTOS side:
> https://github.com/PineconeCode/nuttx/blob/song-u1/include/nuttx/serial/uart_rpmsg.h
> https://github.com/PineconeCode/nuttx/blob/song-u1/drivers/serial/uart_rpmsg.c
>
> Maybe, we can consider how to unify these two implementation into one.
Yes sure.
>
>>>
using some wrapper functions available here:
https://github.com/STMicroelectronics/STM32CubeMP1/tree/master/Middlewares/Third_Party/OpenAMP
Best Regards
Arnaud
>
>> Signed-off-by: Arnaud Pouliquen
>> Signed-off-by: Fabien Dessenne
>> ---
>> drivers/tty/Kconfig | 9 ++
>> drivers/tty/Makefile| 1 +
>> drivers/tty/rpmsg_tty.c | 309
>>
>> 3 files changed, 319 insertions(+)
>> create mode 100644 drivers/tty/rpmsg_tty.c
>>
>> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
>> index 0840d27..42696e6 100644
>> --- a/drivers/tty/Kconfig
>> +++ b/drivers/tty/Kconfig
>> @@ -441,4 +441,1
Re: [PATCH 2/2] tty: add rpmsg driver
On Fri, Apr 5, 2019 at 8:33 PM Arnaud Pouliquen wrote:
>
>
>
> On 4/5/19 12:12 PM, xiang xiao wrote:
> > On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> > wrote:
> >>
> >> Hello Xiang,
> >>
> >>
> >> On 4/3/19 2:47 PM, xiang xiao wrote:
> >>> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> >>> wrote:
>
> This driver exposes a standard tty interface on top of the rpmsg
> framework through the "rpmsg-tty-channel" rpmsg service.
>
> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> per rpmsg endpoint.
>
> >>>
> >>> How to support multi-instances from the same remoteproc instance? I
> >>> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> >>> only one channel can be created for each remote side.
> >> The driver is multi-instance based on muti-endpoints on top of the
> >> "rpmsg-tty-channel" service.
> >> On remote side you just have to call rpmsg_create_ept with destination
> >> address set to ANY. The result is a NS service announcement that trigs a
> >> probe with a new endpoint.
> >> You can find code example for the remote side here:
> >> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
> >
> > Demo code create two uarts(huart0 and huart1), and both use the same
> > channel name( "rpmsg-tty-channel").
> > But rpmsg_create_channel in kernel will complain the duplication:
> > /* make sure a similar channel doesn't already exist */
> > tmp = rpmsg_find_device(dev, chinfo);
> > if (tmp) {
> > /* decrement the matched device's refcount back */
> > put_device(tmp);
> > dev_err(dev, "channel %s:%x:%x already exist\n",
> >chinfo->name, chinfo->src, chinfo->dst);
> > return NULL;
> > }
> > Do you have some local change not upstream yet?
> Nothing is missing. There is no complain as the function
> rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
> to the remote ept address) is different.
>
Yes, you are right.
> If i well remember you have also a similar implementation of the service
> on your side. Do you see any incompatibility with your implementation?
>
Our implementation is similar to yours, but has two major difference:
1.Each instance has a different channel name but share the same prefix
"rpmsg-tty*", the benefit is that:
a.Device name(/dev/tty*) is derived from rpmsg-tty*, instead the
random /dev/ttyRPMSGx
b.Don't need tty_idr to allocate the unique device id
2.Each transfer need get response from peer to avoid the buffer
overflow. This is very important if the peer use pull
model(read/write) instead of push model(callback).
Here is the patch for kernel side:
https://github.com/xiaoxiang781216/linux/commit/f04d2386eb11e0781f0eb47d99fae838abf7eb53
https://github.com/xiaoxiang781216/linux/commit/1a41be362d916eb9104bf21065cb38a0a63d2e91
And RTOS side:
https://github.com/PineconeCode/nuttx/blob/song-u1/include/nuttx/serial/uart_rpmsg.h
https://github.com/PineconeCode/nuttx/blob/song-u1/drivers/serial/uart_rpmsg.c
Maybe, we can consider how to unify these two implementation into one.
> >
> >> using some wrapper functions available here:
> >> https://github.com/STMicroelectronics/STM32CubeMP1/tree/master/Middlewares/Third_Party/OpenAMP
> >>
> >>
> >> Best Regards
> >> Arnaud
> >>
> >>>
> Signed-off-by: Arnaud Pouliquen
> Signed-off-by: Fabien Dessenne
> ---
> drivers/tty/Kconfig | 9 ++
> drivers/tty/Makefile| 1 +
> drivers/tty/rpmsg_tty.c | 309
>
> 3 files changed, 319 insertions(+)
> create mode 100644 drivers/tty/rpmsg_tty.c
>
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index 0840d27..42696e6 100644
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -441,4 +441,13 @@ config VCC
> depends on SUN_LDOMS
> help
> Support for Sun logical domain consoles.
> +
> +config RPMSG_TTY
> + tristate "RPMSG tty driver"
> + depends on RPMSG
> + help
> + Say y here to export rpmsg endpoints as tty devices, usually
> found
> + in /dev/ttyRPMSGx.
> + This makes it possible for user-space programs to send and
> receive
> + rpmsg messages as a standard tty protocol.
> endif # TTY
> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> index c72cafd..90a98a2 100644
> --- a/drivers/tty/Makefile
> +++ b/drivers/tty/Makefile
> @@ -33,5 +33,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
> obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
> obj-$(CONFIG_VCC) += vcc.o
> +obj-$(CONFIG_RPMSG_TTY)+= rpmsg_tty.o
>
>
Re: [PATCH 2/2] tty: add rpmsg driver
Hi Johan,
On 03/04/2019 10:44 AM, Johan Hovold wrote:
> On Thu, Mar 21, 2019 at 04:47:19PM +0100, Fabien Dessenne wrote:
>> This driver exposes a standard tty interface on top of the rpmsg
>> framework through the "rpmsg-tty-channel" rpmsg service.
>>
>> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
>> per rpmsg endpoint.
>>
>> Signed-off-by: Arnaud Pouliquen
>> Signed-off-by: Fabien Dessenne
>> ---
>> drivers/tty/Kconfig | 9 ++
>> drivers/tty/Makefile| 1 +
>> drivers/tty/rpmsg_tty.c | 309
>>
>> 3 files changed, 319 insertions(+)
>> create mode 100644 drivers/tty/rpmsg_tty.c
>> +static int rpmsg_tty_write(struct tty_struct *tty, const unsigned char *buf,
>> + int total)
>> +{
>> +int count, ret = 0;
>> +const unsigned char *tbuf;
>> +struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index);
>> +struct rpmsg_device *rpdev;
>> +int msg_size;
>> +
>> +if (!cport) {
>> +dev_err(tty->dev, "cannot get cport\n");
>> +return -ENODEV;
>> +}
>> +
>> +rpdev = cport->rpdev;
>> +
>> +dev_dbg(&rpdev->dev, "%s: send message from tty->index = %d\n",
>> +__func__, tty->index);
>> +
>> +if (!buf) {
>> +dev_err(&rpdev->dev, "buf shouldn't be null.\n");
>> +return -ENOMEM;
>> +}
>> +
>> +msg_size = rpmsg_get_buf_payload_size(rpdev->ept);
>> +if (msg_size < 0)
>> +return msg_size;
>> +
>> +count = total;
>> +tbuf = buf;
>> +do {
>> +/* send a message to our remote processor */
>> +ret = rpmsg_send(rpdev->ept, (void *)tbuf,
>> + min(count, msg_size));
> Just a drive-by comment; it looks like rpmsg_send() may block, but
> the tty-driver write() callback must never sleep.
OK, I will use rpmsg_trysend() which does not block.
BR,
Fabien
>
>> +if (ret) {
>> +dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", ret);
>> +return ret;
>> +}
>> +
>> +if (count > msg_size) {
>> +count -= msg_size;
>> +tbuf += msg_size;
>> +} else {
>> +count = 0;
>> +}
>> +} while (count > 0);
>> +
>> +return total;
>> +}
> Johan
Re: [PATCH 2/2] tty: add rpmsg driver
On 4/5/19 12:12 PM, xiang xiao wrote:
> On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
> wrote:
>>
>> Hello Xiang,
>>
>>
>> On 4/3/19 2:47 PM, xiang xiao wrote:
>>> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
>>> wrote:
This driver exposes a standard tty interface on top of the rpmsg
framework through the "rpmsg-tty-channel" rpmsg service.
This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
per rpmsg endpoint.
>>>
>>> How to support multi-instances from the same remoteproc instance? I
>>> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
>>> only one channel can be created for each remote side.
>> The driver is multi-instance based on muti-endpoints on top of the
>> "rpmsg-tty-channel" service.
>> On remote side you just have to call rpmsg_create_ept with destination
>> address set to ANY. The result is a NS service announcement that trigs a
>> probe with a new endpoint.
>> You can find code example for the remote side here:
>> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
>
> Demo code create two uarts(huart0 and huart1), and both use the same
> channel name( "rpmsg-tty-channel").
> But rpmsg_create_channel in kernel will complain the duplication:
> /* make sure a similar channel doesn't already exist */
> tmp = rpmsg_find_device(dev, chinfo);
> if (tmp) {
> /* decrement the matched device's refcount back */
> put_device(tmp);
> dev_err(dev, "channel %s:%x:%x already exist\n",
>chinfo->name, chinfo->src, chinfo->dst);
> return NULL;
> }
> Do you have some local change not upstream yet?
Nothing is missing. There is no complain as the function
rpmsg_device_match returns 0, because the chinfo->dst (that corresponds
to the remote ept address) is different.
If i well remember you have also a similar implementation of the service
on your side. Do you see any incompatibility with your implementation?
>
>> using some wrapper functions available here:
>> https://github.com/STMicroelectronics/STM32CubeMP1/tree/master/Middlewares/Third_Party/OpenAMP
>>
>>
>> Best Regards
>> Arnaud
>>
>>>
Signed-off-by: Arnaud Pouliquen
Signed-off-by: Fabien Dessenne
---
drivers/tty/Kconfig | 9 ++
drivers/tty/Makefile| 1 +
drivers/tty/rpmsg_tty.c | 309
3 files changed, 319 insertions(+)
create mode 100644 drivers/tty/rpmsg_tty.c
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 0840d27..42696e6 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -441,4 +441,13 @@ config VCC
depends on SUN_LDOMS
help
Support for Sun logical domain consoles.
+
+config RPMSG_TTY
+ tristate "RPMSG tty driver"
+ depends on RPMSG
+ help
+ Say y here to export rpmsg endpoints as tty devices, usually
found
+ in /dev/ttyRPMSGx.
+ This makes it possible for user-space programs to send and
receive
+ rpmsg messages as a standard tty protocol.
endif # TTY
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index c72cafd..90a98a2 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -33,5 +33,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
obj-$(CONFIG_VCC) += vcc.o
+obj-$(CONFIG_RPMSG_TTY)+= rpmsg_tty.o
obj-y += ipwireless/
diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
new file mode 100644
index 000..9c2a629
--- /dev/null
+++ b/drivers/tty/rpmsg_tty.c
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
+ * Authors: Arnaud Pouliquen for
STMicroelectronics.
+ * Fabien Dessenne for
STMicroelectronics.
+ * Derived from the imx-rpmsg and omap-rpmsg implementations.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_TTY_RPMSG 32
+
+static DEFINE_IDR(tty_idr);/* tty instance id */
+static DEFINE_MUTEX(idr_lock); /* protects tty_idr */
+
+static struct tty_driver *rpmsg_tty_driver;
+
+struct rpmsg_tty_port {
+ struct tty_port port; /* TTY port data */
+ unsigned intid; /* TTY rpmsg index */
+ spinlock_t rx_lock; /* message reception lock */
+ struct rpmsg_device *rpdev; /* rpmsg device */
+};
+
+static int rpmsg_tty_c
Re: [PATCH 2/2] tty: add rpmsg driver
On Fri, Apr 5, 2019 at 12:14 AM Arnaud Pouliquen
wrote:
>
> Hello Xiang,
>
>
> On 4/3/19 2:47 PM, xiang xiao wrote:
> > On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> > wrote:
> >>
> >> This driver exposes a standard tty interface on top of the rpmsg
> >> framework through the "rpmsg-tty-channel" rpmsg service.
> >>
> >> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> >> per rpmsg endpoint.
> >>
> >
> > How to support multi-instances from the same remoteproc instance? I
> > saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> > only one channel can be created for each remote side.
> The driver is multi-instance based on muti-endpoints on top of the
> "rpmsg-tty-channel" service.
> On remote side you just have to call rpmsg_create_ept with destination
> address set to ANY. The result is a NS service announcement that trigs a
> probe with a new endpoint.
> You can find code example for the remote side here:
> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
Demo code create two uarts(huart0 and huart1), and both use the same
channel name( "rpmsg-tty-channel").
But rpmsg_create_channel in kernel will complain the duplication:
/* make sure a similar channel doesn't already exist */
tmp = rpmsg_find_device(dev, chinfo);
if (tmp) {
/* decrement the matched device's refcount back */
put_device(tmp);
dev_err(dev, "channel %s:%x:%x already exist\n",
chinfo->name, chinfo->src, chinfo->dst);
return NULL;
}
Do you have some local change not upstream yet?
> using some wrapper functions available here:
> https://github.com/STMicroelectronics/STM32CubeMP1/tree/master/Middlewares/Third_Party/OpenAMP
>
>
> Best Regards
> Arnaud
>
> >
> >> Signed-off-by: Arnaud Pouliquen
> >> Signed-off-by: Fabien Dessenne
> >> ---
> >> drivers/tty/Kconfig | 9 ++
> >> drivers/tty/Makefile| 1 +
> >> drivers/tty/rpmsg_tty.c | 309
> >>
> >> 3 files changed, 319 insertions(+)
> >> create mode 100644 drivers/tty/rpmsg_tty.c
> >>
> >> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> >> index 0840d27..42696e6 100644
> >> --- a/drivers/tty/Kconfig
> >> +++ b/drivers/tty/Kconfig
> >> @@ -441,4 +441,13 @@ config VCC
> >> depends on SUN_LDOMS
> >> help
> >> Support for Sun logical domain consoles.
> >> +
> >> +config RPMSG_TTY
> >> + tristate "RPMSG tty driver"
> >> + depends on RPMSG
> >> + help
> >> + Say y here to export rpmsg endpoints as tty devices, usually
> >> found
> >> + in /dev/ttyRPMSGx.
> >> + This makes it possible for user-space programs to send and
> >> receive
> >> + rpmsg messages as a standard tty protocol.
> >> endif # TTY
> >> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> >> index c72cafd..90a98a2 100644
> >> --- a/drivers/tty/Makefile
> >> +++ b/drivers/tty/Makefile
> >> @@ -33,5 +33,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
> >> obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
> >> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
> >> obj-$(CONFIG_VCC) += vcc.o
> >> +obj-$(CONFIG_RPMSG_TTY)+= rpmsg_tty.o
> >>
> >> obj-y += ipwireless/
> >> diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
> >> new file mode 100644
> >> index 000..9c2a629
> >> --- /dev/null
> >> +++ b/drivers/tty/rpmsg_tty.c
> >> @@ -0,0 +1,309 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
> >> + * Authors: Arnaud Pouliquen for
> >> STMicroelectronics.
> >> + * Fabien Dessenne for
> >> STMicroelectronics.
> >> + * Derived from the imx-rpmsg and omap-rpmsg implementations.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#define MAX_TTY_RPMSG 32
> >> +
> >> +static DEFINE_IDR(tty_idr);/* tty instance id */
> >> +static DEFINE_MUTEX(idr_lock); /* protects tty_idr */
> >> +
> >> +static struct tty_driver *rpmsg_tty_driver;
> >> +
> >> +struct rpmsg_tty_port {
> >> + struct tty_port port; /* TTY port data */
> >> + unsigned intid; /* TTY rpmsg index */
> >> + spinlock_t rx_lock; /* message reception lock */
> >> + struct rpmsg_device *rpdev; /* rpmsg device */
> >> +};
> >> +
> >> +static int rpmsg_tty_cb(struct rpmsg_device *rpdev, void *data, int len,
> >> + void *priv, u32 src)
> >> +{
> >> + int space;
> >> + unsigned char *cbuf;
> >> + struct rpmsg_tty_port *cport = dev_get_drvdata(&rpdev->dev);
> >> +
> >> + dev_dbg(&rpdev->dev, "msg(<- src 0x%x) len %d\n", src, len);
> >> +
> >> + print_hex_dump_debug(__func__, DUMP_PREFIX_NONE, 16, 1, data, len,
>
Re: [PATCH 2/2] tty: add rpmsg driver
Hello Xiang,
On 4/3/19 2:47 PM, xiang xiao wrote:
> On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne
> wrote:
>>
>> This driver exposes a standard tty interface on top of the rpmsg
>> framework through the "rpmsg-tty-channel" rpmsg service.
>>
>> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
>> per rpmsg endpoint.
>>
>
> How to support multi-instances from the same remoteproc instance? I
> saw that the channel name is fixed to "rpmsg-tty-channel" which mean
> only one channel can be created for each remote side.
The driver is multi-instance based on muti-endpoints on top of the
"rpmsg-tty-channel" service.
On remote side you just have to call rpmsg_create_ept with destination
address set to ANY. The result is a NS service announcement that trigs a
probe with a new endpoint.
You can find code example for the remote side here:
https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_TTY_echo/Src/main.c
using some wrapper functions available here:
https://github.com/STMicroelectronics/STM32CubeMP1/tree/master/Middlewares/Third_Party/OpenAMP
Best Regards
Arnaud
>
>> Signed-off-by: Arnaud Pouliquen
>> Signed-off-by: Fabien Dessenne
>> ---
>> drivers/tty/Kconfig | 9 ++
>> drivers/tty/Makefile| 1 +
>> drivers/tty/rpmsg_tty.c | 309
>>
>> 3 files changed, 319 insertions(+)
>> create mode 100644 drivers/tty/rpmsg_tty.c
>>
>> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
>> index 0840d27..42696e6 100644
>> --- a/drivers/tty/Kconfig
>> +++ b/drivers/tty/Kconfig
>> @@ -441,4 +441,13 @@ config VCC
>> depends on SUN_LDOMS
>> help
>> Support for Sun logical domain consoles.
>> +
>> +config RPMSG_TTY
>> + tristate "RPMSG tty driver"
>> + depends on RPMSG
>> + help
>> + Say y here to export rpmsg endpoints as tty devices, usually found
>> + in /dev/ttyRPMSGx.
>> + This makes it possible for user-space programs to send and receive
>> + rpmsg messages as a standard tty protocol.
>> endif # TTY
>> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
>> index c72cafd..90a98a2 100644
>> --- a/drivers/tty/Makefile
>> +++ b/drivers/tty/Makefile
>> @@ -33,5 +33,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
>> obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
>> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
>> obj-$(CONFIG_VCC) += vcc.o
>> +obj-$(CONFIG_RPMSG_TTY)+= rpmsg_tty.o
>>
>> obj-y += ipwireless/
>> diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
>> new file mode 100644
>> index 000..9c2a629
>> --- /dev/null
>> +++ b/drivers/tty/rpmsg_tty.c
>> @@ -0,0 +1,309 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
>> + * Authors: Arnaud Pouliquen for
>> STMicroelectronics.
>> + * Fabien Dessenne for STMicroelectronics.
>> + * Derived from the imx-rpmsg and omap-rpmsg implementations.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define MAX_TTY_RPMSG 32
>> +
>> +static DEFINE_IDR(tty_idr);/* tty instance id */
>> +static DEFINE_MUTEX(idr_lock); /* protects tty_idr */
>> +
>> +static struct tty_driver *rpmsg_tty_driver;
>> +
>> +struct rpmsg_tty_port {
>> + struct tty_port port; /* TTY port data */
>> + unsigned intid; /* TTY rpmsg index */
>> + spinlock_t rx_lock; /* message reception lock */
>> + struct rpmsg_device *rpdev; /* rpmsg device */
>> +};
>> +
>> +static int rpmsg_tty_cb(struct rpmsg_device *rpdev, void *data, int len,
>> + void *priv, u32 src)
>> +{
>> + int space;
>> + unsigned char *cbuf;
>> + struct rpmsg_tty_port *cport = dev_get_drvdata(&rpdev->dev);
>> +
>> + dev_dbg(&rpdev->dev, "msg(<- src 0x%x) len %d\n", src, len);
>> +
>> + print_hex_dump_debug(__func__, DUMP_PREFIX_NONE, 16, 1, data, len,
>> +true);
>> +
>> + /* Flush the recv-ed none-zero data to tty node */
>> + if (!len)
>> + return -EINVAL;
>> +
>> + spin_lock(&cport->rx_lock);
>> + space = tty_prepare_flip_string(&cport->port, &cbuf, len);
>> + if (space <= 0) {
>> + dev_err(&rpdev->dev, "No memory for
>> tty_prepare_flip_string\n");
>> + spin_unlock(&cport->rx_lock);
>> + return -ENOMEM;
>> + }
>> +
>> + if (space != len)
>> + dev_warn(&rpdev->dev, "Trunc buffer from %d to %d\n",
>> +len, space);
>> +
>> + memcpy(cbuf, data, space);
>> + tty_flip_buffer_push(&cport->port);
>> + spin_unlock(&cport->rx_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static int rpmsg_tty_install(struct tty_dri
Re: [PATCH 2/2] tty: add rpmsg driver
On Thu, Mar 21, 2019 at 11:48 PM Fabien Dessenne wrote:
>
> This driver exposes a standard tty interface on top of the rpmsg
> framework through the "rpmsg-tty-channel" rpmsg service.
>
> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> per rpmsg endpoint.
>
How to support multi-instances from the same remoteproc instance? I
saw that the channel name is fixed to "rpmsg-tty-channel" which mean
only one channel can be created for each remote side.
> Signed-off-by: Arnaud Pouliquen
> Signed-off-by: Fabien Dessenne
> ---
> drivers/tty/Kconfig | 9 ++
> drivers/tty/Makefile| 1 +
> drivers/tty/rpmsg_tty.c | 309
>
> 3 files changed, 319 insertions(+)
> create mode 100644 drivers/tty/rpmsg_tty.c
>
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index 0840d27..42696e6 100644
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -441,4 +441,13 @@ config VCC
> depends on SUN_LDOMS
> help
> Support for Sun logical domain consoles.
> +
> +config RPMSG_TTY
> + tristate "RPMSG tty driver"
> + depends on RPMSG
> + help
> + Say y here to export rpmsg endpoints as tty devices, usually found
> + in /dev/ttyRPMSGx.
> + This makes it possible for user-space programs to send and receive
> + rpmsg messages as a standard tty protocol.
> endif # TTY
> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> index c72cafd..90a98a2 100644
> --- a/drivers/tty/Makefile
> +++ b/drivers/tty/Makefile
> @@ -33,5 +33,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
> obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
> obj-$(CONFIG_VCC) += vcc.o
> +obj-$(CONFIG_RPMSG_TTY)+= rpmsg_tty.o
>
> obj-y += ipwireless/
> diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
> new file mode 100644
> index 000..9c2a629
> --- /dev/null
> +++ b/drivers/tty/rpmsg_tty.c
> @@ -0,0 +1,309 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
> + * Authors: Arnaud Pouliquen for
> STMicroelectronics.
> + * Fabien Dessenne for STMicroelectronics.
> + * Derived from the imx-rpmsg and omap-rpmsg implementations.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define MAX_TTY_RPMSG 32
> +
> +static DEFINE_IDR(tty_idr);/* tty instance id */
> +static DEFINE_MUTEX(idr_lock); /* protects tty_idr */
> +
> +static struct tty_driver *rpmsg_tty_driver;
> +
> +struct rpmsg_tty_port {
> + struct tty_port port; /* TTY port data */
> + unsigned intid; /* TTY rpmsg index */
> + spinlock_t rx_lock; /* message reception lock */
> + struct rpmsg_device *rpdev; /* rpmsg device */
> +};
> +
> +static int rpmsg_tty_cb(struct rpmsg_device *rpdev, void *data, int len,
> + void *priv, u32 src)
> +{
> + int space;
> + unsigned char *cbuf;
> + struct rpmsg_tty_port *cport = dev_get_drvdata(&rpdev->dev);
> +
> + dev_dbg(&rpdev->dev, "msg(<- src 0x%x) len %d\n", src, len);
> +
> + print_hex_dump_debug(__func__, DUMP_PREFIX_NONE, 16, 1, data, len,
> +true);
> +
> + /* Flush the recv-ed none-zero data to tty node */
> + if (!len)
> + return -EINVAL;
> +
> + spin_lock(&cport->rx_lock);
> + space = tty_prepare_flip_string(&cport->port, &cbuf, len);
> + if (space <= 0) {
> + dev_err(&rpdev->dev, "No memory for
> tty_prepare_flip_string\n");
> + spin_unlock(&cport->rx_lock);
> + return -ENOMEM;
> + }
> +
> + if (space != len)
> + dev_warn(&rpdev->dev, "Trunc buffer from %d to %d\n",
> +len, space);
> +
> + memcpy(cbuf, data, space);
> + tty_flip_buffer_push(&cport->port);
> + spin_unlock(&cport->rx_lock);
> +
> + return 0;
> +}
> +
> +static int rpmsg_tty_install(struct tty_driver *driver, struct tty_struct
> *tty)
> +{
> + struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index);
> +
> + if (!cport) {
> + dev_err(tty->dev, "cannot get cport\n");
> + return -ENODEV;
> + }
> +
> + return tty_port_install(&cport->port, driver, tty);
> +}
> +
> +static int rpmsg_tty_open(struct tty_struct *tty, struct file *filp)
> +{
> + return tty_port_open(tty->port, tty, filp);
> +}
> +
> +static void rpmsg_tty_close(struct tty_struct *tty, struct file *filp)
> +{
> + return tty_port_close(tty->port, tty, filp);
> +}
> +
> +static int rpmsg_tty_write(struct tty_struct *tty, const unsigned char *buf,
> + int total)
> +{
> + int count, ret = 0;
> + const unsigned char *tbuf;
> +
Re: [PATCH 2/2] tty: add rpmsg driver
On Thu, Mar 21, 2019 at 04:47:19PM +0100, Fabien Dessenne wrote:
> This driver exposes a standard tty interface on top of the rpmsg
> framework through the "rpmsg-tty-channel" rpmsg service.
>
> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> per rpmsg endpoint.
>
> Signed-off-by: Arnaud Pouliquen
> Signed-off-by: Fabien Dessenne
> ---
> drivers/tty/Kconfig | 9 ++
> drivers/tty/Makefile| 1 +
> drivers/tty/rpmsg_tty.c | 309
>
> 3 files changed, 319 insertions(+)
> create mode 100644 drivers/tty/rpmsg_tty.c
> +static int rpmsg_tty_write(struct tty_struct *tty, const unsigned char *buf,
> +int total)
> +{
> + int count, ret = 0;
> + const unsigned char *tbuf;
> + struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index);
> + struct rpmsg_device *rpdev;
> + int msg_size;
> +
> + if (!cport) {
> + dev_err(tty->dev, "cannot get cport\n");
> + return -ENODEV;
> + }
> +
> + rpdev = cport->rpdev;
> +
> + dev_dbg(&rpdev->dev, "%s: send message from tty->index = %d\n",
> + __func__, tty->index);
> +
> + if (!buf) {
> + dev_err(&rpdev->dev, "buf shouldn't be null.\n");
> + return -ENOMEM;
> + }
> +
> + msg_size = rpmsg_get_buf_payload_size(rpdev->ept);
> + if (msg_size < 0)
> + return msg_size;
> +
> + count = total;
> + tbuf = buf;
> + do {
> + /* send a message to our remote processor */
> + ret = rpmsg_send(rpdev->ept, (void *)tbuf,
> + min(count, msg_size));
Just a drive-by comment; it looks like rpmsg_send() may block, but
the tty-driver write() callback must never sleep.
> + if (ret) {
> + dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", ret);
> + return ret;
> + }
> +
> + if (count > msg_size) {
> + count -= msg_size;
> + tbuf += msg_size;
> + } else {
> + count = 0;
> + }
> + } while (count > 0);
> +
> + return total;
> +}
Johan
