On 13/03/2019 13:34, Dan Carpenter wrote:
> On Wed, Mar 13, 2019 at 01:24:04PM +0100, Armando Miraglia wrote:
>> Running Lindent on the mt7621-spi.c file in drivers/staging I noticed that
>> the
>> file contained style issues. This change attempts to address such style
>> problems.
>>
>
> Don't run lindent. I think checkpatch.pl has a --fix option that might
> be better, but once the code is merged then our standard become much
> higher for follow up patches.
>
>> Signed-off-by: Armando Miraglia <ar...@google.com>
>> ---
>> NOTE: resend this patch to include all mainteners listed by
>> get_mantainers.pl.
>> drivers/staging/mt7621-spi/spi-mt7621.c | 27 +++++++++++++------------
>> 1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
>> b/drivers/staging/mt7621-spi/spi-mt7621.c
>> index b509f9fe3346..03d53845f8c5 100644
>> --- a/drivers/staging/mt7621-spi/spi-mt7621.c
>> +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
>> @@ -52,14 +52,14 @@
>> #define MT7621_LSB_FIRST BIT(3)
>>
>> struct mt7621_spi {
>> - struct spi_master *master;
>> - void __iomem *base;
>> - unsigned int sys_freq;
>> - unsigned int speed;
>> - struct clk *clk;
>> - int pending_write;
>> -
>> - struct mt7621_spi_ops *ops;
>> + struct spi_master *master;
>> + void __iomem *base;
>> + unsigned int sys_freq;
>> + unsigned int speed;
>> + struct clk *clk;
>> + int pending_write;
>> +
>> + struct mt7621_spi_ops *ops;
>
> The original is fine. I don't encourage people to do fancy indenting
> with their local variable declarations inside functions but for a struct
> the declarations aren't going to change a lot so people can get fancy
> if they want.
>
> The problem with a local is if you need to add a new variable then you
> have to re-indent a bunch of unrelated lines or have one out of
> alignment line. Most people know this intuitively so they don't get
> fancy.
>
>> };
>>
>> static inline struct mt7621_spi *spidev_to_mt7621_spi(struct spi_device
>> *spi)
>> @@ -303,7 +303,7 @@ static int mt7621_spi_setup(struct spi_device *spi)
>> struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
>>
>> if ((spi->max_speed_hz == 0) ||
>> - (spi->max_speed_hz > (rs->sys_freq / 2)))
>> + (spi->max_speed_hz > (rs->sys_freq / 2)))
>
> Yeah. Lindent is correct here.
>
>> spi->max_speed_hz = (rs->sys_freq / 2);
>>
>> if (spi->max_speed_hz < (rs->sys_freq / 4097)) {
>> @@ -316,9 +316,10 @@ static int mt7621_spi_setup(struct spi_device *spi)
>> }
>>
>> static const struct of_device_id mt7621_spi_match[] = {
>> - { .compatible = "ralink,mt7621-spi" },
>> + {.compatible = "ralink,mt7621-spi"},
>
> The original was better.
>
>> {},
>> };
>> +
>> MODULE_DEVICE_TABLE(of, mt7621_spi_match);
>
> No need for a blank. These are closely related.
>
>>
>> static int mt7621_spi_probe(struct platform_device *pdev)
>> @@ -408,9 +409,9 @@ MODULE_ALIAS("platform:" DRIVER_NAME);
>>
>> static struct platform_driver mt7621_spi_driver = {
>> .driver = {
>> - .name = DRIVER_NAME,
>> - .of_match_table = mt7621_spi_match,
>> - },
>> + .name = DRIVER_NAME,
>> + .of_match_table = mt7621_spi_match,
>> + },
>
> The new indenting is very wrong.
>
Fair enough, I was too fast providing my Reviewed-by tag :-/
