diff mbox series

[v4,2/3] dt-bindings: sc16is7xx: Add compatible line for XR20M1172 UART

Message ID 20240419124506.1531035-3-rilian.la.te@ya.ru
State Not Applicable
Headers show
Series None | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Konstantin Pugin April 19, 2024, 12:45 p.m. UTC 
From: Konstantin Pugin <ria.freelander@gmail.com>

Add EXAR XR20M1172 UART compatible line into devicetree documentation.

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
---
 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski April 19, 2024, 1:32 p.m. UTC | #1 
On 19/04/2024 14:45, Konstantin Pugin wrote:
> From: Konstantin Pugin <ria.freelander@gmail.com>
> 
> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
> 
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
> Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
> ---
>  Documentation/devicetree/bindings/serial/nxp,sc16is7xx.ya

This is fourth change, no cover letter, no changelog. Patch is trivial
but you do not make it easier to understand what is happening here.

Please provide proper changelog under ---.

(If you wrote changelog somewhere else and then decided not to send it
to us, it is like there was no changelog. I literally do not have it in
my inbox).


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof
Konstantin P. April 19, 2024, 1:49 p.m. UTC | #2 
On Fri, Apr 19, 2024 at 4:32 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 19/04/2024 14:45, Konstantin Pugin wrote:
> > From: Konstantin Pugin <ria.freelander@gmail.com>
> >
> > Add EXAR XR20M1172 UART compatible line into devicetree documentation.
> >
> > Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
> > Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/serial/nxp,sc16is7xx.ya
>
> This is fourth change, no cover letter, no changelog. Patch is trivial
> but you do not make it easier to understand what is happening here.
>
> Please provide proper changelog under ---.
>
> (If you wrote changelog somewhere else and then decided not to send it
> to us, it is like there was no changelog. I literally do not have it in
> my inbox).
>
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
>
> ---
>
> This is an automated instruction, just in case, because many review tags
> are being ignored. If you know the process, you can skip it (please do
> not feel offended by me posting it here - no bad intentions intended).
> If you do not know the process, here is a short explanation:
>
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions, under or above your Signed-off-by tag. Tag is "received", when
> provided in a message replied to you on the mailing list. Tools like b4
> can help here. However, there's no need to repost patches *only* to add
> the tags. The upstream maintainer will do that for tags received on the
> version they apply.
>
> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
>
> Best regards,
> Krzysztof
>

I am sorry, I used git send-email, and send all 3 patches and cover
letter. I do not know why it was not ended up in your mailbox.
Link to all patches (version 4)
https://lore.kernel.org/linux-serial/20240419124506.1531035-1-rilian.la.te@ya.ru/.

Here is a git send-email log for cover letter:

```
$ git send-email v4*.patch --cc-cmd='./scripts/get_maintainer.pl
--norolestats v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
v4-0000-cover-letter.patch
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch
v4-0002-dt-bindings-sc16is7xx-Add-compatible-line-for-XR2.patch
v4-0003-serial-sc16is7xx-add-support-for-EXAR-XR20M1172-U.patch
To whom should the emails be sent (if anyone)?
Message-ID to be used as In-Reply-To for the first email (if any)?
(mbox) Adding cc: Konstantin Pugin <ria.freelander@gmail.com> from
line 'From: Konstantin Pugin <ria.freelander@gmail.com>'
./scripts/get_maintainer.pl: file 'v4-0000-cover-letter.patch' doesn't
appear to be a patch.  Add -f to options?
(cc-cmd) Adding cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
from: './scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Jiri Slaby <jirislaby@kernel.org> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Hugo Villeneuve <hvilleneuve@dimonoff.com> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Andy Shevchenko
<andriy.shevchenko@linux.intel.com> from: './scripts/get_maintainer.pl
--norolestats v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Lech Perczak <lech.perczak@camlingroup.com> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?=
<ilpo.jarvinen@linux.intel.com> from: './scripts/get_maintainer.pl
--norolestats v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: Thomas Gleixner <tglx@linutronix.de> from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: linux-kernel@vger.kernel.org from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'
(cc-cmd) Adding cc: linux-serial@vger.kernel.org from:
'./scripts/get_maintainer.pl --norolestats
v4-0001-serial-sc16is7xx-announce-support-of-SER_RS485_RT.patch'

From: Konstantin Pugin <rilian.la.te@ya.ru>
To:
Cc: Konstantin Pugin <ria.freelander@gmail.com>,
       Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
       Jiri Slaby <jirislaby@kernel.org>,
       Hugo Villeneuve <hvilleneuve@dimonoff.com>,
       Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
       Lech Perczak <lech.perczak@camlingroup.com>,
       =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
       Thomas Gleixner <tglx@linutronix.de>,
       linux-kernel@vger.kernel.org,
       linux-serial@vger.kernel.org
Subject: [PATCH v4 0/3] add support for EXAR XR20M1172 UART
Date: Fri, 19 Apr 2024 15:45:00 +0300
Message-Id: <20240419124506.1531035-1-rilian.la.te@ya.ru>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

   The Cc list above has been expanded by additional
   addresses found in the patch commit message. By default
   send-email prompts before sending whenever this occurs.
   This behavior is controlled by the sendemail.confirm
   configuration setting.

   For additional information, run 'git send-email --help'.
   To retain the current behavior, but squelch this message,
   run 'git config --global sendemail.confirm auto'.

Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll):
OK. Log says:
Server: smtp.yandex.ru
MAIL FROM:<rilian.la.te@ya.ru>
RCPT TO:<ria.freelander@gmail.com>
RCPT TO:<gregkh@linuxfoundation.org>
RCPT TO:<jirislaby@kernel.org>
RCPT TO:<hvilleneuve@dimonoff.com>
RCPT TO:<andriy.shevchenko@linux.intel.com>
RCPT TO:<lech.perczak@camlingroup.com>
RCPT TO:<ilpo.jarvinen@linux.intel.com>
RCPT TO:<tglx@linutronix.de>
RCPT TO:<linux-kernel@vger.kernel.org>
RCPT TO:<linux-serial@vger.kernel.org>
From: Konstantin Pugin <rilian.la.te@ya.ru>
To:
Cc: Konstantin Pugin <ria.freelander@gmail.com>,
       Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
       Jiri Slaby <jirislaby@kernel.org>,
       Hugo Villeneuve <hvilleneuve@dimonoff.com>,
       Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
       Lech Perczak <lech.perczak@camlingroup.com>,
       =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
       Thomas Gleixner <tglx@linutronix.de>,
       linux-kernel@vger.kernel.org,
       linux-serial@vger.kernel.org
Subject: [PATCH v4 0/3] add support for EXAR XR20M1172 UART
Date: Fri, 19 Apr 2024 15:45:00 +0300
Message-Id: <20240419124506.1531035-1-rilian.la.te@ya.ru>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Result: 250

```
Krzysztof Kozlowski April 19, 2024, 2:01 p.m. UTC | #3 
On 19/04/2024 15:49, Konstantin P. wrote:
>> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
>> versions, under or above your Signed-off-by tag. Tag is "received", when
>> provided in a message replied to you on the mailing list. Tools like b4
>> can help here. However, there's no need to repost patches *only* to add
>> the tags. The upstream maintainer will do that for tags received on the
>> version they apply.
>>
>> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
>>
>> Best regards,
>> Krzysztof
>>
> 
> I am sorry, I used git send-email, and send all 3 patches and cover
> letter. I do not know why it was not ended up in your mailbox.
> Link to all patches (version 4)
> https://lore.kernel.org/linux-serial/20240419124506.1531035-1-rilian.la.te@ya.ru/.
> 
> Here is a git send-email log for cover letter:

So read the log and you will see that it does not send to everyone.
That's how cc-cmd works.

I would suggest to switch to b4 or patman.

Best regards,
Krzysztof
Conor Dooley April 19, 2024, 2:07 p.m. UTC | #4 
On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
> From: Konstantin Pugin <ria.freelander@gmail.com>
> 
> Add EXAR XR20M1172 UART compatible line into devicetree documentation.

What you're doing is obvious from the diff, why this exar device is
related to the nxp devices is what should be mentioned here.

Thanks,
Conor.

> 
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
> Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
> ---
>  Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> index 5dec15b7e7c3..c4bedf23368b 100644
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> @@ -12,6 +12,7 @@ maintainers:
>  properties:
>    compatible:
>      enum:
> +      - exar,xr20m1172
>        - nxp,sc16is740
>        - nxp,sc16is741
>        - nxp,sc16is750
> -- 
> 2.34.1
>
Konstantin P. April 19, 2024, 2:17 p.m. UTC | #5 
On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
> > From: Konstantin Pugin <ria.freelander@gmail.com>
> >
> > Add EXAR XR20M1172 UART compatible line into devicetree documentation.
>
> What you're doing is obvious from the diff, why this exar device is
> related to the nxp devices is what should be mentioned here.
>
> Thanks,
> Conor.

It is already mentioned in cover letter and in previous patches in the
series. Do I need to repeat it in DTS patch?
If so, I will do it.

Citation from my cover letter:

EXAR XR20M1172 UART is mostly SC16IS762-compatible, but
it has additional register which can change UART multiplier
to 4x and 8x, similar to UPF_MAGIC_MULTIPLIER does. So, I used this
flag to guard access to its specific DLD register. It seems than
other EXAR SPI UART modules also have this register, but I tested
only XR20M1172.
Yes, in datasheet this register is called "DLD - Divisor Fractional"
or "DLD - Divisor Fractional Register", calling depends on datasheet
version.

Also, comparision from NXP itself:
http://www.bdtic.com/download/nxp/75017168.pdf (pp12-13 is about XR20M1172).

> >
> > Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
> > Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > index 5dec15b7e7c3..c4bedf23368b 100644
> > --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > @@ -12,6 +12,7 @@ maintainers:
> >  properties:
> >    compatible:
> >      enum:
> > +      - exar,xr20m1172
> >        - nxp,sc16is740
> >        - nxp,sc16is741
> >        - nxp,sc16is750
> > --
> > 2.34.1
> >
Krzysztof Kozlowski April 19, 2024, 2:24 p.m. UTC | #6 
On 19/04/2024 16:17, Konstantin P. wrote:
> On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <conor@kernel.org> wrote:
>>
>> On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
>>> From: Konstantin Pugin <ria.freelander@gmail.com>
>>>
>>> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
>>
>> What you're doing is obvious from the diff, why this exar device is
>> related to the nxp devices is what should be mentioned here.
>>
>> Thanks,
>> Conor.
> 
> It is already mentioned in cover letter and in previous patches in the
> series. Do I need to repeat it in DTS patch?
> If so, I will do it.
> 
> Citation from my cover letter:
> 
> EXAR XR20M1172 UART is mostly SC16IS762-compatible, but
> it has additional register which can change UART multiplier
> to 4x and 8x, similar to UPF_MAGIC_MULTIPLIER does. So, I used this
> flag to guard access to its specific DLD register. It seems than
> other EXAR SPI UART modules also have this register, but I tested
> only XR20M1172.
> Yes, in datasheet this register is called "DLD - Divisor Fractional"
> or "DLD - Divisor Fractional Register", calling depends on datasheet
> version.

Commits must stand on their own. Cover letter is not merged. This is the
place where you add new hardware, so here you describe and explain the
hardware.

Best regards,
Krzysztof
Konstantin P. April 19, 2024, 2:34 p.m. UTC | #7 
On Fri, Apr 19, 2024 at 5:24 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 19/04/2024 16:17, Konstantin P. wrote:
> > On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> On Fri, Apr 19, 2024 at 03:45:02PM +0300, Konstantin Pugin wrote:
> >>> From: Konstantin Pugin <ria.freelander@gmail.com>
> >>>
> >>> Add EXAR XR20M1172 UART compatible line into devicetree documentation.
> >>
> >> What you're doing is obvious from the diff, why this exar device is
> >> related to the nxp devices is what should be mentioned here.
> >>
> >> Thanks,
> >> Conor.
> >
> > It is already mentioned in cover letter and in previous patches in the
> > series. Do I need to repeat it in DTS patch?
> > If so, I will do it.
> >
> > Citation from my cover letter:
> >
> > EXAR XR20M1172 UART is mostly SC16IS762-compatible, but
> > it has additional register which can change UART multiplier
> > to 4x and 8x, similar to UPF_MAGIC_MULTIPLIER does. So, I used this
> > flag to guard access to its specific DLD register. It seems than
> > other EXAR SPI UART modules also have this register, but I tested
> > only XR20M1172.
> > Yes, in datasheet this register is called "DLD - Divisor Fractional"
> > or "DLD - Divisor Fractional Register", calling depends on datasheet
> > version.
>
> Commits must stand on their own. Cover letter is not merged. This is the
> place where you add new hardware, so here you describe and explain the
> hardware.

It is also described in patch 3 in the series. I need to repeat this
description in patch 2 too?

Cite from patch 3:

XR20M1172 register set is mostly compatible with SC16IS762, but it has
a support for additional division rates of UART with special DLD register.

> Best regards,
> Krzysztof
>
Andy Shevchenko April 19, 2024, 2:49 p.m. UTC | #8 
On Fri, Apr 19, 2024 at 04:24:16PM +0200, Krzysztof Kozlowski wrote:
> On 19/04/2024 16:17, Konstantin P. wrote:
> > On Fri, Apr 19, 2024 at 5:08 PM Conor Dooley <conor@kernel.org> wrote:

...

> Commits must stand on their own. Cover letter is not merged.

While the first is true, the second one depends on the maintainer and tooling.
For the patch series some maintainers either already use custom, or started
using `b4` feature to make cover letter to be a merge commit-like message.

Yet, it's not the case for TTY subsystem AFAIK.

> This is the place where you add new hardware, so here you describe and
> explain the hardware.
Andy Shevchenko April 19, 2024, 2:52 p.m. UTC | #9 
On Fri, Apr 19, 2024 at 05:34:44PM +0300, Konstantin P. wrote:
> On Fri, Apr 19, 2024 at 5:24 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On 19/04/2024 16:17, Konstantin P. wrote:

...

> > Commits must stand on their own. Cover letter is not merged. This is the
> > place where you add new hardware, so here you describe and explain the
> > hardware.
> 
> It is also described in patch 3 in the series. I need to repeat this
> description in patch 2 too?
> 
> Cite from patch 3:
> 
> XR20M1172 register set is mostly compatible with SC16IS762, but it has
> a support for additional division rates of UART with special DLD register.

The point is, if I got it correctly, to have a few words in the description
of the DT binding itself, so whoever reads the bindings (w/o even accessing
the Git history of the project) may understand this.
Conor Dooley April 19, 2024, 2:56 p.m. UTC | #10 
On Fri, Apr 19, 2024 at 05:52:38PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 19, 2024 at 05:34:44PM +0300, Konstantin P. wrote:
> > On Fri, Apr 19, 2024 at 5:24 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > On 19/04/2024 16:17, Konstantin P. wrote:
> 
> ...
> 
> > > Commits must stand on their own. Cover letter is not merged. This is the
> > > place where you add new hardware, so here you describe and explain the
> > > hardware.
> > 
> > It is also described in patch 3 in the series. I need to repeat this
> > description in patch 2 too?
> > 
> > Cite from patch 3:
> > 
> > XR20M1172 register set is mostly compatible with SC16IS762, but it has
> > a support for additional division rates of UART with special DLD register.
> 
> The point is, if I got it correctly, to have a few words in the description
> of the DT binding itself, so whoever reads the bindings (w/o even accessing
> the Git history of the project) may understand this.

Yes, each patch must independently justify the change - although in this
case it is about the history not people reading the bindings as I was
talking about the commit message and not the contents of the diff.
It's especially true when I only get sent the bindings patch, although
from other messages in this thread that may not have been intentional.

Thanks,
Conor.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
index 5dec15b7e7c3..c4bedf23368b 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -12,6 +12,7 @@  maintainers:
 properties:
   compatible:
     enum:
+      - exar,xr20m1172
       - nxp,sc16is740
       - nxp,sc16is741
       - nxp,sc16is750