diff mbox series

[1/3] clocksource/drivers/sysctr: Add an optional property

Message ID 20190621082838.12630-1-Anson.Huang@nxp.com (mailing list archive)
State New, archived
Headers show
Series [1/3] clocksource/drivers/sysctr: Add an optional property | expand

Commit Message

Anson Huang June 21, 2019, 8:28 a.m. UTC 
From: Anson Huang <Anson.Huang@nxp.com>

This patch adds an optional property "clock-frequency" to pass
the system counter frequency value to kernel system counter
driver and indicate the driver to skip of_clk operations, this
is to support those platforms using platform driver model for
clock driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Thomas Gleixner June 23, 2019, 10:46 a.m. UTC | #1 
Anson,

On Fri, 21 Jun 2019, Anson.Huang@nxp.com wrote:

> Subject : [PATCH 1/3] clocksource/drivers/sysctr: Add an optional property

That subject line is not really informative. From Documentation:

     The ``summary phrase`` in the email's Subject should concisely
     describe the patch which that email contains.

That means that it should tell which property it adds so it's immediately
clear what this is about. Something like:

 Subject: clocksource/drivers/sysctr: Add optional clock-frequency property

Hmm?

> From: Anson Huang <Anson.Huang@nxp.com>
> 
> This patch adds an optional property "clock-frequency" to pass

Please read Documentation/process/submitting-patches.rst and search for
'This patch'

> the system counter frequency value to kernel system counter
> driver and indicate the driver to skip of_clk operations, this
> is to support those platforms using platform driver model for
> clock driver.

That sentence does not parse. Please structure your changelog in the
following order:

   1) Context or problem

   2) Detailed analysis (if applicable and necessary)

   3) Short description of the solution (the rest is obvious from the patch
      itself).

So something like this (assumed I decoded the above correctly):

   Systems which use the system counter with the platform driver model
   require the clock frequency to be supplied via device tree.

   This is necessary as in the platform driver model the of_clk operations
   do not work correctly because LENGHTY EXPLANATION WHY ...

   Add the optinal clock-frequency to the device tree bindings of the NXP
   system counter so the frequency can be handed in and the of_clk
   operations can be skipped.

The important part is the missing LENGTHY EXPLANATION WHY. I can't fill
that in because you did not provide that information.

Thanks,

	tglx
Anson Huang June 23, 2019, 11:31 a.m. UTC | #2 
Hi, Thomas
	Thanks for the useful comment, I will resend the patch with improvement.

Anson.

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Sunday, June 23, 2019 6:47 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: daniel.lezcano@linaro.org; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; l.stach@pengutronix.de; Abel Vesa
> <abel.vesa@nxp.com>; ccaione@baylibre.com; angus@akkea.ca;
> andrew.smirnov@gmail.com; agx@sigxcpu.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> property
> 
> Anson,
> 
> On Fri, 21 Jun 2019, Anson.Huang@nxp.com wrote:
> 
> > Subject : [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> > property
> 
> That subject line is not really informative. From Documentation:
> 
>      The ``summary phrase`` in the email's Subject should concisely
>      describe the patch which that email contains.
> 
> That means that it should tell which property it adds so it's immediately clear
> what this is about. Something like:
> 
>  Subject: clocksource/drivers/sysctr: Add optional clock-frequency property
> 
> Hmm?
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > This patch adds an optional property "clock-frequency" to pass
> 
> Please read Documentation/process/submitting-patches.rst and search for
> 'This patch'
> 
> > the system counter frequency value to kernel system counter driver and
> > indicate the driver to skip of_clk operations, this is to support
> > those platforms using platform driver model for clock driver.
> 
> That sentence does not parse. Please structure your changelog in the
> following order:
> 
>    1) Context or problem
> 
>    2) Detailed analysis (if applicable and necessary)
> 
>    3) Short description of the solution (the rest is obvious from the patch
>       itself).
> 
> So something like this (assumed I decoded the above correctly):
> 
>    Systems which use the system counter with the platform driver model
>    require the clock frequency to be supplied via device tree.
> 
>    This is necessary as in the platform driver model the of_clk operations
>    do not work correctly because LENGHTY EXPLANATION WHY ...
> 
>    Add the optinal clock-frequency to the device tree bindings of the NXP
>    system counter so the frequency can be handed in and the of_clk
>    operations can be skipped.
> 
> The important part is the missing LENGTHY EXPLANATION WHY. I can't fill
> that in because you did not provide that information.
> 
> Thanks,
> 
> 	tglx
Thomas Gleixner June 23, 2019, 11:34 a.m. UTC | #3 
Anson,

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Sun, 23 Jun 2019, Anson Huang wrote:

> Hi, Thomas
> 	Thanks for the useful comment, I will resend the patch with improvement.
> 
> Anson.
> 
> > -----Original Message-----
> > From: Thomas Gleixner <tglx@linutronix.de>
> > Sent: Sunday, June 23, 2019 6:47 PM
> > To: Anson Huang <anson.huang@nxp.com>
> > Cc: daniel.lezcano@linaro.org; robh+dt@kernel.org; mark.rutland@arm.com;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; l.stach@pengutronix.de; Abel Vesa
> > <abel.vesa@nxp.com>; ccaione@baylibre.com; angus@akkea.ca;
> > andrew.smirnov@gmail.com; agx@sigxcpu.org; linux-
> > kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> > property

Also please fix your mailer to NOT copy the full mail header into the
reply. That's absolutely useless.

> > 
> > Anson,
> > 
> > On Fri, 21 Jun 2019, Anson.Huang@nxp.com wrote:
> > 
> > > Subject : [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> > > property

And finally please trim your replies, so the uninteresting parts are gone.

Thanks,

	tglx
Anson Huang June 23, 2019, 12:05 p.m. UTC | #4 
Hi, Thomas

> Anson,
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 
> A: No.
> Q: Should I include quotations after my reply?
> 
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdaringf
> ireball.net%2F2007%2F07%2Fon_top&amp;data=02%7C01%7Canson.huang
> %40nxp.com%7C658d12e0d65a401034d208d6f7cecc86%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C1%7C636968864908338236&amp;sdata=%2F0%
> 2B9DIT2HVuVFgLvhW7QFFNAXrRqbcTi9%2BJcasgOv08%3D&amp;reserved=0
> 

Thanks for these info.

> On Sun, 23 Jun 2019, Anson Huang wrote:
> 
> > Hi, Thomas
> > 	Thanks for the useful comment, I will resend the patch with
> improvement.
> >
> > Anson.
> > 
> Also please fix your mailer to NOT copy the full mail header into the reply.
> That's absolutely useless.

Sure, thanks for reminder.

> 
> > >
> > > Anson,
> > >
> > > On Fri, 21 Jun 2019, Anson.Huang@nxp.com wrote:
> > >
> > > > Subject : [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> > > > property
> 
> And finally please trim your replies, so the uninteresting parts are gone.
>

OK.

Thanks,
Anson.
 
> Thanks,
> 
> 	tglx
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..c9907a0 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -14,6 +14,11 @@  Required properties:
 - clocks : 	    Specifies the counter clock.
 - clock-names: 	    Specifies the clock's name of this module
 
+Optional properties:
+
+- clock-frequency : Specifies system counter clock frequency and indicates system
+		    counter driver to skip clock operations.
+
 Example:
 
 	system_counter: timer@306a0000 {
@@ -22,4 +27,5 @@  Example:
 		clocks = <&clk_8m>;
 		clock-names = "per";
 		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+		clock-frequency = <8333333>;
 	};