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

[Anson Huang]

Hi, Thomas
	Thanks for the useful comment, I will resend the patch with improvement.

Anson.

> -----Original Message-----
> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Sent: Sunday, June 23, 2019 6:47 PM
> To: Anson Huang <anson.huang@xxxxxxx>
> Cc: daniel.lezcano@xxxxxxxxxx; robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx;
> shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> festevam@xxxxxxxxx; l.stach@xxxxxxxxxxxxxx; Abel Vesa
> <abel.vesa@xxxxxxx>; ccaione@xxxxxxxxxxxx; angus@xxxxxxxx;
> andrew.smirnov@xxxxxxxxx; agx@xxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: Re: [PATCH 1/3] clocksource/drivers/sysctr: Add an optional
> property
> 
> Anson,
> 
> On Fri, 21 Jun 2019, Anson.Huang@xxxxxxx 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@xxxxxxx>
> >
> > 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