On Tue, 2021-03-30 at 16:23 +0800, Moriis Ku wrote:
> From: Morris <[email protected]>
>
> Signed-off-by: Morris <[email protected]>
> ---
> 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...
