| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <57c63984-ddf1-6cd7-40a3-aadf9405da5a@foss.st.com>
Date: Fri, 1 Apr 2022 15:27:36 +0200
From: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
To: Deepak Kumar Singh <quic_deesin@...cinc.com>,
Bjorn Andersson <bjorn.andersson@...aro.org>
CC: <swboyd@...omium.org>, <quic_clew@...cinc.com>,
<mathieu.poirier@...aro.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>,
<linux-remoteproc@...r.kernel.org>, Andy Gross <agross@...nel.org>,
Ohad Ben-Cohen <ohad@...ery.com>
Subject: Re: [PATCH V2 2/3] rpmsg: glink: Add support to handle signals
command
On 3/23/22 08:20, Deepak Kumar Singh wrote:
>
> On 3/12/2022 2:39 AM, Bjorn Andersson wrote:
>> On Tue 18 Jan 13:43 CST 2022, Deepak Kumar Singh wrote:
>>
>>> Remote peripherals send signal notifications over glink with
>>> commandID 15.
>>>
>>> Add support to send and receive the signal command and convert the
>>> signals
>>> from NATIVE to TIOCM while receiving and vice versa while sending.
>>>
>>> Signed-off-by: Chris Lew <quic_clew@...cinc.com>
>> Co-developed-by: seems appropriate here, or you need to ensure the
>> author remains Chris, as his S-o-b comes first.
>>
>>> Signed-off-by: Deepak Kumar Singh <quic_deesin@...cinc.com>
>>> ---
>>> drivers/rpmsg/qcom_glink_native.c | 77
>>> +++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 77 insertions(+)
>>>
>>> diff --git a/drivers/rpmsg/qcom_glink_native.c
>>> b/drivers/rpmsg/qcom_glink_native.c
>>> index 3f377a7..d673d65 100644
>>> --- a/drivers/rpmsg/qcom_glink_native.c
>>> +++ b/drivers/rpmsg/qcom_glink_native.c
>>> @@ -17,6 +17,7 @@
>>> #include <linux/rpmsg.h>
>>> #include <linux/sizes.h>
>>> #include <linux/slab.h>
>>> +#include <linux/termios.h>
>>> #include <linux/workqueue.h>
>>> #include <linux/mailbox_client.h>
>>> @@ -205,9 +206,16 @@ static const struct rpmsg_endpoint_ops
>>> glink_endpoint_ops;
>>> #define RPM_CMD_TX_DATA_CONT 12
>>> #define RPM_CMD_READ_NOTIF 13
>>> #define RPM_CMD_RX_DONE_W_REUSE 14
>>> +#define RPM_CMD_SIGNALS 15
>>> #define GLINK_FEATURE_INTENTLESS BIT(1)
>>> +#define NATIVE_DTR_SIG BIT(31)
>> Seems reasonable to prefix these with GLINK_, perhaps GLINK_SIGNAL_DTR?
>>
>>> +#define NATIVE_CTS_SIG BIT(30)
>>> +#define NATIVE_CD_SIG BIT(29)
>>> +#define NATIVE_RI_SIG BIT(28)
>>> +#define SIG_MASK 0x0fff;
>>> +
>>> static void qcom_glink_rx_done_work(struct work_struct *work);
>>> static struct glink_channel *qcom_glink_alloc_channel(struct
>>> qcom_glink *glink,
>>> @@ -1003,6 +1011,70 @@ static int qcom_glink_rx_open_ack(struct
>>> qcom_glink *glink, unsigned int lcid)
>>> return 0;
>>> }
>>> +/**
>>> + * qcom_glink_set_flow_control() - convert a signal cmd to wire
>>> format and
>>> + * transmit
>>> + * @ept: Rpmsg endpoint for channel.
>>> + * @enable: True/False - enable or disable flow control
>> "enable flow control" sounds sufficient (i.e. no need for True/False)
>> part.
>>
>> Regards,
>> Bjorn
>
> There are some user space clients which require both flow control on and
> off (DTR high/low).
>
> So i guess true and false both are needed.
>
>>> + *
>>> + * Return: 0 on success or standard Linux error code.
>>> + */
>>> +static int qcom_glink_set_flow_control(struct rpmsg_endpoint *ept,
>>> bool enable)
>>> +{
>>> + struct glink_channel *channel = to_glink_channel(ept);
>>> + struct qcom_glink *glink = channel->glink;
>>> + struct glink_msg msg;
>>> + u32 sigs;
>>> +
>>> + /**
>>> + * convert signals from TIOCM to NATIVE
>>> + * sigs = TIOCM_DTR|TIOCM_RTS
>>> + */
>>> + if (enable)
>>> + sigs |= NATIVE_DTR_SIG | NATIVE_CTS_SIG;
>>> + else
>>> + sigs |= ~(NATIVE_DTR_SIG | NATIVE_CTS_SIG);
>>> +
>>> + msg.cmd = cpu_to_le16(RPM_CMD_SIGNALS);
>>> + msg.param1 = cpu_to_le16(channel->lcid);
>>> + msg.param2 = cpu_to_le32(sigs);
>>> +
>>> + return qcom_glink_tx(glink, &msg, sizeof(msg), NULL, 0, true);
>>> +}
>>> +
>>> +static int qcom_glink_handle_signals(struct qcom_glink *glink,
>>> + unsigned int rcid, unsigned int sigs)
>>> +{
>>> + struct glink_channel *channel;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&glink->idr_lock, flags);
>>> + channel = idr_find(&glink->rcids, rcid);
>>> + spin_unlock_irqrestore(&glink->idr_lock, flags);
>>> + if (!channel) {
>>> + dev_err(glink->dev, "signal for non-existing channel\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + if (!channel->ept.sig_cb)
>>> + return 0;
>>> +
>>> + /* convert signals from NATIVE to TIOCM */
>>> + if (sigs & NATIVE_DTR_SIG)
Regarding specs seems that DTR is from the DTE (Data Terminal Equipment)
to the DCE (Data Communication Equipement)
NATIVE_DSR_SIG instead?
Regards
Arnaud
>>> + sigs |= TIOCM_DSR;
>>> + if (sigs & NATIVE_CTS_SIG)
>>> + sigs |= TIOCM_CTS;
>>> + if (sigs & NATIVE_CD_SIG)
>>> + sigs |= TIOCM_CD;
>>> + if (sigs & NATIVE_RI_SIG)
>>> + sigs |= TIOCM_RI;
>>> + sigs &= SIG_MASK;
>>> +
>>> + channel->ept.sig_cb(channel->ept.rpdev, channel->ept.priv, sigs);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static irqreturn_t qcom_glink_native_intr(int irq, void *data)
>>> {
>>> struct qcom_glink *glink = data;
>>> @@ -1067,6 +1139,10 @@ static irqreturn_t qcom_glink_native_intr(int
>>> irq, void *data)
>>> qcom_glink_handle_intent_req_ack(glink, param1, param2);
>>> qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>>> break;
>>> + case RPM_CMD_SIGNALS:
>>> + qcom_glink_handle_signals(glink, param1, param2);
>>> + qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>>> + break;
>>> default:
>>> dev_err(glink->dev, "unhandled rx cmd: %d\n", cmd);
>>> ret = -EINVAL;
>>> @@ -1442,6 +1518,7 @@ static const struct rpmsg_endpoint_ops
>>> glink_endpoint_ops = {
>>> .sendto = qcom_glink_sendto,
>>> .trysend = qcom_glink_trysend,
>>> .trysendto = qcom_glink_trysendto,
>>> + .set_flow_control = qcom_glink_set_flow_control,
>>> };
>>> static void qcom_glink_rpdev_release(struct device *dev)
>>> --
>>> 2.7.4
>>>
Powered by blists - more mailing lists