All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-mips@vger.kernel.org,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Andreas Färber" <afaerber@suse.de>,
	"Saravanan Sekar" <sravanhome@gmail.com>,
	"Parthiban Nallathambi" <pn@denx.de>,
	linux-actions@lists.infradead.org
Subject: Re: [PATCH v3 1/4] clk: actions: Terminate clk_div_table with sentinel element
Date: Sat, 5 Feb 2022 13:58:39 +0100	[thread overview]
Message-ID: <Yf50f7wbJi9PnUNV@latitude> (raw)
In-Reply-To: <5567d063-39b3-adfb-6556-d9973fa2074b@amsat.org>

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On Sat, Feb 05, 2022 at 01:15:46PM +0100, Philippe Mathieu-Daudé wrote:
> On 5/2/22 01:52, Jonathan Neuschäfer wrote:
> > In order that the end of a clk_div_table can be detected, it must be
> > terminated with a sentinel element (.div = 0).
> > 
> > In owl-s900.s, the { 0, 8 } element was probably meant to be just that,
> > so this patch changes { 0, 8 } to { 0, 0 }.
> > 
> > Fixes: d47317ca4ade1 ("clk: actions: Add S700 SoC clock support")
> > Fixes: d85d20053e195 ("clk: actions: Add S900 SoC clock support")
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > ---
[...]
> >   static struct clk_div_table rmii_div_table[] = {
> >   	{0, 4},   {1, 10},
> > +	{0, 0},
> 
> Again, clearer to use:
> 
>         { /* sentinel */ }
> 
> (without the trailing comma, to enforce this is the last entry).

This style is locally consistent in the file, so anyone who reads the
file will encounter { 0, 0 } elements anyway.

So, I think it makes sense to:

- Keep this patch as is (except for the trailing comma, I'll remove that)
- Add a second patch that replaces all the sentinel elements with
  { /* sentinel */ }

This patch can go into stable branches, and the style cleanup doesn't
have to.


Best regards,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-mips@vger.kernel.org,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Andreas Färber" <afaerber@suse.de>,
	"Saravanan Sekar" <sravanhome@gmail.com>,
	"Parthiban Nallathambi" <pn@denx.de>,
	linux-actions@lists.infradead.org
Subject: Re: [PATCH v3 1/4] clk: actions: Terminate clk_div_table with sentinel element
Date: Sat, 5 Feb 2022 13:58:39 +0100	[thread overview]
Message-ID: <Yf50f7wbJi9PnUNV@latitude> (raw)
In-Reply-To: <5567d063-39b3-adfb-6556-d9973fa2074b@amsat.org>


[-- Attachment #1.1: Type: text/plain, Size: 1333 bytes --]

On Sat, Feb 05, 2022 at 01:15:46PM +0100, Philippe Mathieu-Daudé wrote:
> On 5/2/22 01:52, Jonathan Neuschäfer wrote:
> > In order that the end of a clk_div_table can be detected, it must be
> > terminated with a sentinel element (.div = 0).
> > 
> > In owl-s900.s, the { 0, 8 } element was probably meant to be just that,
> > so this patch changes { 0, 8 } to { 0, 0 }.
> > 
> > Fixes: d47317ca4ade1 ("clk: actions: Add S700 SoC clock support")
> > Fixes: d85d20053e195 ("clk: actions: Add S900 SoC clock support")
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > ---
[...]
> >   static struct clk_div_table rmii_div_table[] = {
> >   	{0, 4},   {1, 10},
> > +	{0, 0},
> 
> Again, clearer to use:
> 
>         { /* sentinel */ }
> 
> (without the trailing comma, to enforce this is the last entry).

This style is locally consistent in the file, so anyone who reads the
file will encounter { 0, 0 } elements anyway.

So, I think it makes sense to:

- Keep this patch as is (except for the trailing comma, I'll remove that)
- Add a second patch that replaces all the sentinel elements with
  { /* sentinel */ }

This patch can go into stable branches, and the style cleanup doesn't
have to.


Best regards,
Jonathan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-02-05 12:59 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-05  0:52 [PATCH v3 0/4] clk drivers: Terminate clk_div_table with sentinel element Jonathan Neuschäfer
2022-02-05  0:52 ` Jonathan Neuschäfer
2022-02-05  0:52 ` [PATCH v3 1/4] clk: actions: " Jonathan Neuschäfer
2022-02-05  0:52   ` Jonathan Neuschäfer
2022-02-05 12:15   ` Philippe Mathieu-Daudé
2022-02-05 12:15     ` Philippe Mathieu-Daudé
2022-02-05 12:58     ` Jonathan Neuschäfer [this message]
2022-02-05 12:58       ` Jonathan Neuschäfer
2022-02-05  0:52 ` [PATCH v3 2/4] clk: loongson1: " Jonathan Neuschäfer
2022-02-05  0:52   ` Jonathan Neuschäfer
2022-02-05 12:17   ` Philippe Mathieu-Daudé
2022-02-05 12:17     ` Philippe Mathieu-Daudé
2022-02-05 12:59     ` Jonathan Neuschäfer
2022-02-05 12:59       ` Jonathan Neuschäfer
2022-02-05  0:52 ` [PATCH v3 3/4] clk: hisilicon: " Jonathan Neuschäfer
2022-02-05  0:52   ` Jonathan Neuschäfer
2022-02-05  0:52 ` [PATCH v3 4/4] clk: clps711x: " Jonathan Neuschäfer
2022-02-05  0:52   ` Jonathan Neuschäfer
2022-02-05 12:13   ` Philippe Mathieu-Daudé
2022-02-05 12:13     ` Philippe Mathieu-Daudé
2022-02-05 13:07     ` Jonathan Neuschäfer
2022-02-05 13:07       ` Jonathan Neuschäfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yf50f7wbJi9PnUNV@latitude \
    --to=j.neuschaefer@gmx.net \
    --cc=afaerber@suse.de \
    --cc=f4bug@amsat.org \
    --cc=linux-actions@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=pn@denx.de \
    --cc=sboyd@kernel.org \
    --cc=sravanhome@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.