[PATCH] virtio-can: rephrase notification mechanism

[PATCH] virtio-can: rephrase notification mechanism

[Matias Ezequiel Vara Larsen]

Rephrase the notification mechanism.

Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
---
* This patch applies on top of virtio-1.4, which has not been released
  yet.
---
 device-types/can/description.tex | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/device-types/can/description.tex b/device-types/can/description.tex
index 2511d9c..2931aa9 100644
--- a/device-types/can/description.tex
+++ b/device-types/can/description.tex
@@ -128,13 +128,12 @@ \subsubsection{Controller Mode}\label{sec:Device Types / CAN Device / Device Ope
 \end{lstlisting}
 
 If the transition succeeded the \field{result} is VIRTIO_CAN_RESULT_OK
-otherwise it is VIRTIO_CAN_RESULT_NOT_OK. If a status update is
-necessary, the device updates the configuration \field{status} before
-marking the request used. As the configuration \field{status} change is
-caused by a request from the driver the device is allowed to omit the
-configuration change notification here. The device marks the request
-used when the CAN controller has finalized the transition to the
-requested controller mode.
+otherwise it is VIRTIO_CAN_RESULT_NOT_OK. If a status update is necessary, the
+device updates the configuration \field{status} before marking the request
+used. The device may omit the configuration change notification as the
+configuration \field{status} change is requested by the driver. The device
+marks the request used when the CAN controller has finalized the transition to
+the requested controller mode.
 
 On transition to the STOPPED state the device cancels all CAN messages
 already pending for transmission and marks them as used with

base-commit: 37c6a406678a5ee891fdf5671298cb4fcfa517f2
-- 
2.42.0

Re: [PATCH] virtio-can: rephrase notification mechanism

[Stefano Garzarella]

On Tue, May 21, 2024 at 04:12:45PM GMT, Matias Ezequiel Vara Larsen wrote:
>Rephrase the notification mechanism.

I'd suggest to explain here what are we clarifying with the rephrase.
Sorry, I'm not a CAN expert, so I don't understand the difference 
between the two versions.

Stefano

>
>Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
>---
>* This patch applies on top of virtio-1.4, which has not been released
>  yet.
>---
> device-types/can/description.tex | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
>diff --git a/device-types/can/description.tex b/device-types/can/description.tex
>index 2511d9c..2931aa9 100644
>--- a/device-types/can/description.tex
>+++ b/device-types/can/description.tex
>@@ -128,13 +128,12 @@ \subsubsection{Controller Mode}\label{sec:Device Types / CAN Device / Device Ope
> \end{lstlisting}
>
> If the transition succeeded the \field{result} is VIRTIO_CAN_RESULT_OK
>-otherwise it is VIRTIO_CAN_RESULT_NOT_OK. If a status update is
>-necessary, the device updates the configuration \field{status} before
>-marking the request used. As the configuration \field{status} change is
>-caused by a request from the driver the device is allowed to omit the
>-configuration change notification here. The device marks the request
>-used when the CAN controller has finalized the transition to the
>-requested controller mode.
>+otherwise it is VIRTIO_CAN_RESULT_NOT_OK. If a status update is necessary, the
>+device updates the configuration \field{status} before marking the request
>+used. The device may omit the configuration change notification as the
>+configuration \field{status} change is requested by the driver. The device
>+marks the request used when the CAN controller has finalized the transition to
>+the requested controller mode.
>
> On transition to the STOPPED state the device cancels all CAN messages
> already pending for transmission and marks them as used with
>
>base-commit: 37c6a406678a5ee891fdf5671298cb4fcfa517f2
>-- 
>2.42.0
>
>

Re: [PATCH] virtio-can: rephrase notification mechanism

[Harald Mommer]

Hello,

exactly this is my problem here also. For me this change looks like 
saying exactly the same thing just in different (not better) words.

Thinking still about the replacement "from the driver" => "by the 
driver". Was the original "from the driver" correct? My native language 
is German and of course it can be that I made a grammar mistake applying 
German grammar to an English text. Absolutely unclear to me.

But if it's not that I have as a non-native speaker of English in the 
moment no idea where the improvement is.

Regards
Harald

On 05.06.24 10:46, Stefano Garzarella wrote:
> On Tue, May 21, 2024 at 04:12:45PM GMT, Matias Ezequiel Vara Larsen 
> wrote:
>> Rephrase the notification mechanism.
>
> I'd suggest to explain here what are we clarifying with the rephrase.
> Sorry, I'm not a CAN expert, so I don't understand the difference 
> between the two versions.
>
> Stefano
>
>>
>> Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
>> ---
>> * This patch applies on top of virtio-1.4, which has not been released
>>  yet.
>> ---
>> device-types/can/description.tex | 13 ++++++-------
>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/device-types/can/description.tex 
>> b/device-types/can/description.tex
>> index 2511d9c..2931aa9 100644
>> --- a/device-types/can/description.tex
>> +++ b/device-types/can/description.tex
>> @@ -128,13 +128,12 @@ \subsubsection{Controller 
>> Mode}\label{sec:Device Types / CAN Device / Device Ope
>> \end{lstlisting}
>>
>> If the transition succeeded the \field{result} is VIRTIO_CAN_RESULT_OK
>> -otherwise it is VIRTIO_CAN_RESULT_NOT_OK. If a status update is
>> -necessary, the device updates the configuration \field{status} before
>> -marking the request used. As the configuration \field{status} change is
>> -caused by a request from the driver the device is allowed to omit the
>> -configuration change notification here. The device marks the request
>> -used when the CAN controller has finalized the transition to the
>> -requested controller mode.
>> +otherwise it is VIRTIO_CAN_RESULT_NOT_OK. If a status update is 
>> necessary, the
>> +device updates the configuration \field{status} before marking the 
>> request
>> +used. The device may omit the configuration change notification as the
>> +configuration \field{status} change is requested by the driver. The 
>> device
>> +marks the request used when the CAN controller has finalized the 
>> transition to
>> +the requested controller mode.
>>
>> On transition to the STOPPED state the device cancels all CAN messages
>> already pending for transmission and marks them as used with
>>
>> base-commit: 37c6a406678a5ee891fdf5671298cb4fcfa517f2
>> -- 
>> 2.42.0
>>
>>
>
-- 
Dipl.-Ing. Harald Mommer
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

Phone:  +49 (30) 60 98 540-0 <== Zentrale
Fax:    +49 (30) 60 98 540-99
E-Mail: harald.mommer@opensynergy.com

www.opensynergy.com

Handelsregister: Amtsgericht Charlottenburg, HRB 108616B
Geschäftsführer/Managing Director: Regis Adjamah

Re: [PATCH] virtio-can: rephrase notification mechanism

[Matias Ezequiel Vara Larsen]

On Wed, Jun 05, 2024 at 07:55:05PM +0200, Harald Mommer wrote:
> Hello,
> 
> exactly this is my problem here also. For me this change looks like saying
> exactly the same thing just in different (not better) words.
> 
> Thinking still about the replacement "from the driver" => "by the driver".
> Was the original "from the driver" correct? My native language is German and
> of course it can be that I made a grammar mistake applying German grammar to
> an English text. Absolutely unclear to me.
> 
> But if it's not that I have as a non-native speaker of English in the moment
> no idea where the improvement is.
> 
> Regards
> Harald
> 
> On 05.06.24 10:46, Stefano Garzarella wrote:
> > On Tue, May 21, 2024 at 04:12:45PM GMT, Matias Ezequiel Vara Larsen
> > wrote:
> > > Rephrase the notification mechanism.
> > 
> > I'd suggest to explain here what are we clarifying with the rephrase.
> > Sorry, I'm not a CAN expert, so I don't understand the difference
> > between the two versions.
> > 

Thanks both for the review. The only sentence that I wanted to change
is:

"As the configuration \field{status} change is caused by a request from
the driver the device is allowed to omit the configuration change
notification here."

I though it could be rewritten in a more direct way like:

"The device may omit the configuration change notification as the
configuration \field{status} change is requested by the driver. "

But I don't know if that clarifies anything. If not, we can drop this
patch.

Matias

Re: [PATCH] virtio-can: rephrase notification mechanism

[Stefano Garzarella]

On Thu, Jun 06, 2024 at 11:39:44AM GMT, Matias Ezequiel Vara Larsen wrote:
>On Wed, Jun 05, 2024 at 07:55:05PM +0200, Harald Mommer wrote:
>> Hello,
>>
>> exactly this is my problem here also. For me this change looks like saying
>> exactly the same thing just in different (not better) words.
>>
>> Thinking still about the replacement "from the driver" => "by the driver".
>> Was the original "from the driver" correct? My native language is German and
>> of course it can be that I made a grammar mistake applying German grammar to
>> an English text. Absolutely unclear to me.
>>
>> But if it's not that I have as a non-native speaker of English in the moment
>> no idea where the improvement is.
>>
>> Regards
>> Harald
>>
>> On 05.06.24 10:46, Stefano Garzarella wrote:
>> > On Tue, May 21, 2024 at 04:12:45PM GMT, Matias Ezequiel Vara Larsen
>> > wrote:
>> > > Rephrase the notification mechanism.
>> >
>> > I'd suggest to explain here what are we clarifying with the rephrase.
>> > Sorry, I'm not a CAN expert, so I don't understand the difference
>> > between the two versions.
>> >
>
>Thanks both for the review. The only sentence that I wanted to change
>is:
>
>"As the configuration \field{status} change is caused by a request from
>the driver the device is allowed to omit the configuration change
>notification here."
>
>I though it could be rewritten in a more direct way like:
>
>"The device may omit the configuration change notification as the
>configuration \field{status} change is requested by the driver. "
>
>But I don't know if that clarifies anything. If not, we can drop this
>patch.

Yeah, I think is more clear but I suggest to explain it in the commit
description as well.

Thanks,
Stefano

Re: [PATCH] virtio-can: rephrase notification mechanism

[Matias Ezequiel Vara Larsen]

On Tue, Jun 11, 2024 at 10:36:47AM +0200, Stefano Garzarella wrote:
> On Thu, Jun 06, 2024 at 11:39:44AM GMT, Matias Ezequiel Vara Larsen wrote:
> > On Wed, Jun 05, 2024 at 07:55:05PM +0200, Harald Mommer wrote:
> > > Hello,
> > > 
> > > exactly this is my problem here also. For me this change looks like saying
> > > exactly the same thing just in different (not better) words.
> > > 
> > > Thinking still about the replacement "from the driver" => "by the driver".
> > > Was the original "from the driver" correct? My native language is German and
> > > of course it can be that I made a grammar mistake applying German grammar to
> > > an English text. Absolutely unclear to me.
> > > 
> > > But if it's not that I have as a non-native speaker of English in the moment
> > > no idea where the improvement is.
> > > 
> > > Regards
> > > Harald
> > > 
> > > On 05.06.24 10:46, Stefano Garzarella wrote:
> > > > On Tue, May 21, 2024 at 04:12:45PM GMT, Matias Ezequiel Vara Larsen
> > > > wrote:
> > > > > Rephrase the notification mechanism.
> > > >
> > > > I'd suggest to explain here what are we clarifying with the rephrase.
> > > > Sorry, I'm not a CAN expert, so I don't understand the difference
> > > > between the two versions.
> > > >
> > 
> > Thanks both for the review. The only sentence that I wanted to change
> > is:
> > 
> > "As the configuration \field{status} change is caused by a request from
> > the driver the device is allowed to omit the configuration change
> > notification here."
> > 
> > I though it could be rewritten in a more direct way like:
> > 
> > "The device may omit the configuration change notification as the
> > configuration \field{status} change is requested by the driver. "
> > 
> > But I don't know if that clarifies anything. If not, we can drop this
> > patch.
> 
> Yeah, I think is more clear but I suggest to explain it in the commit
> description as well.
> 

Thanks, I will submit a v2 adding that.

Matias