On Tue, Jul 09, 2024 at 04:23:02PM -0400, Frank Li wrote:
Subject: Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()
The subject prefixes in the EDAC subsystem have the following format:
"EDAC/driver: <Sentence starting with a capital letter"
> Pass down fsl_mc_data in help function ddr_in32() and ddr_out32() to
> prepare add iMX9 support. iMX9 have a little difference register layout.
Since everyone is using AI nowadays, you could have it verify your commit
messages. I did this:
| ChatGPT, Please verify and correct the grammar in this english text: "Pass
| down fsl_mc_data in help function ddr_in32() and ddr_out32() to prepare add
| iMX9 support. iMX9 have a little difference register layout."
It replied with:
| "Pass down fsl_mc_data in the helper functions ddr_in32() and ddr_out32() to
| prepare for adding iMX9 support. The iMX9 has a slightly different register
| layout."
|
| Changes made:
| 1. "help function" changed to "helper functions" for clarity.
|
| 2. "prepare add iMX9 support" changed to "prepare for adding iMX9 support" for
| grammatical correctness.
|
| 3. "iMX9 have a little difference register layout" changed to "The iMX9 has
| a slightly different register layout" for subject-verb agreement and
| smoother phrasing.
And this all looks good to me.
With all the cringe we all get from AI, I think it is very useful for
verifying commit messages.
Do that for all your commit messages pls.
Thx.
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> ---
> drivers/edac/fsl_ddr_edac.c | 62 ++++++++++++++++++++++++---------------------
> 1 file changed, 33 insertions(+), 29 deletions(-)
How did you test these patches of yours?
They don't even build!
drivers/edac/fsl_ddr_edac.c: In function 'fsl_mc_err_probe':
drivers/edac/fsl_ddr_edac.c:538:21: error: too few arguments to function 'ddr_in32'
538 | sdram_ctl = ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG);
| ^~~~~~~~
drivers/edac/fsl_ddr_edac.c:38:19: note: declared here
38 | static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
| ^~~~~~~~
make[4]: *** [scripts/Makefile.build:229: drivers/edac/fsl_ddr_edac.o] Error 1
make[3]: *** [scripts/Makefile.build:478: drivers/edac] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:478: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/mnt/kernel/kernel/2nd/linux/Makefile:1936: .] Error 2
make: *** [Makefile:224: __sub-make] Error 2
Before you submit next time, build-test *every* *single* patch of yours and
test the driver with all of them.
This should not ever happen in submission.
Stopping review here.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
