On Tue, 2021-03-30 at 16:23 +0800, Moriis Ku wrote: > From: Morris <sau...@gmail.com> > > Signed-off-by: Morris <sau...@gmail.com> > --- > spi_pack.c | 1506 ++++++++++++++++++++++++++++++++++++++++++++++++++++
You might try to use scripts/checkpatch.pl on this and see if there is anything you want to change to have the code look more like the rest of the kernel. Using ./scripts/checkpatch.pl --strict --fix <patch> from the top of the kernel tree should help quite a bit with making the code layout look more like typical kernel style. > diff --git a/spi_pack.c b/spi_pack.c [] > +static void get_info(struct sunix_sdc_spi_channel *spi_chl, unsigned int > incomeLength, unsigned int * translateLength) > +{ [] > + do > + { > + Address = spi_chl->info.phy2_base_start + > spi_chl->info.memoffset; > + > + } while (false); That's an odd and unnecessary use of a do while. > + memset(TrBuff, 0, SUNIX_SDC_SPI_BUFF); > + TrLength = 0; > + > + pTrHeader->Version = 0x01; > + pTrHeader->CmdResponseEventData = pRxHeader->CmdResponseEventData | > 0x8000; > + pTrHeader->ResponseStatus = nStatus; > + pTrHeader->Length = (nStatus == SDCSPI_STATUS_SUCCESS)?(31 + > (cib_info->spi_number_of_device * 12)):0; > + TrLength = sizeof(SPI_HEADER); Perhaps reorder the patch submission to define the structs first. This can't compile as SPI_HEADER isn't defined SPI_HEADER and PSPI_HEADER should use not use typedefs and use the typical struct spi_header and struct spi_header * > + if (pTrHeader->ResponseStatus == SDCSPI_STATUS_SUCCESS) Hungarian isn't generally used in the kernel. CamelCase isn't generally used either. > + { > + memcpy(&TrBuff[TrLength], spi_chl->info.model_name, 16); > + TrLength += 16; > + TrBuff[TrLength++] = spi_chl->info.bus_number; > + TrBuff[TrLength++] = spi_chl->info.dev_number; > + TrBuff[TrLength++] = spi_chl->info.line; > + TrBuff[TrLength++] = (unsigned char)((Address & 0xff000000) >> > 24); > + TrBuff[TrLength++] = (unsigned char)((Address & 0x00ff0000) >> > 16); > + TrBuff[TrLength++] = (unsigned char)((Address & 0x0000ff00) >> > 8); > + TrBuff[TrLength++] = (unsigned char)((Address & 0x000000ff)); > + TrBuff[TrLength++] = (unsigned char)(spi_chl->info.irq); You might consider using a single char * and increment that and not use TrLength at all and use p - TrBuff when necessary. *p++ = spi_chl->info.bus_number; *p++ = spi_chl->info.dev_number; ... > + TrBuff[TrLength++] = (unsigned > char)((cib_info->spi_significand_of_clock & 0xff000000) >> 24); > + TrBuff[TrLength++] = (unsigned > char)((cib_info->spi_significand_of_clock & 0x00ff0000) >> 16); > + TrBuff[TrLength++] = (unsigned > char)((cib_info->spi_significand_of_clock & 0x0000ff00) >> 8); > + TrBuff[TrLength++] = (unsigned > char)((cib_info->spi_significand_of_clock & 0x000000ff)); Perhaps put_unaligned_le32(cib_info->spi_significant_of_clock, p); p += 4; etc...