Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()

[Frank Li <Frank.li@nxp.com>]

On Thu, Oct 10, 2024 at 05:33:20PM +0200, Borislav Petkov wrote:
> 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@nxp.com>
> > ---
> >  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.

Really sorry about this. I need debug why my check script have not found
this problem.

Frank

>
> Stopping review here.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette