* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-13 12:34 ` Dan Carpenter
0 siblings, 0 replies; 46+ messages in thread
From: Dan Carpenter @ 2019-03-13 12:34 UTC (permalink / raw)
To: Armando Miraglia
Cc: devel, gregkh, linux-kernel, matthias.bgg, Armando Miraglia,
linux-mediatek, sankalpnegi2310, neil, sr, linux-arm-kernel
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 <armax@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.
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-13 12:34 ` Dan Carpenter
0 siblings, 0 replies; 46+ messages in thread
From: Dan Carpenter @ 2019-03-13 12:34 UTC (permalink / raw)
To: Armando Miraglia
Cc: devel, gregkh, linux-kernel, matthias.bgg, Armando Miraglia,
linux-mediatek, sankalpnegi2310, neil, sr, linux-arm-kernel
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 <armax@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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-13 12:34 ` Dan Carpenter
@ 2019-03-13 16:47 ` Matthias Brugger
-1 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-13 16:47 UTC (permalink / raw)
To: Dan Carpenter, Armando Miraglia
Cc: neil, devel, gregkh, linux-kernel, Armando Miraglia,
linux-mediatek, sankalpnegi2310, sr, linux-arm-kernel
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 <armax@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 :-/
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-13 16:47 ` Matthias Brugger
0 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-13 16:47 UTC (permalink / raw)
To: Dan Carpenter, Armando Miraglia
Cc: devel, gregkh, linux-kernel, Armando Miraglia, linux-mediatek,
sankalpnegi2310, neil, sr, linux-arm-kernel
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 <armax@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 :-/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-13 12:34 ` Dan Carpenter
@ 2019-03-14 11:13 ` Armando Miraglia
-1 siblings, 0 replies; 46+ messages in thread
From: Armando Miraglia @ 2019-03-14 11:13 UTC (permalink / raw)
To: Dan Carpenter
Cc: neil, devel, gregkh, linux-kernel, linux-mediatek,
sankalpnegi2310, matthias.bgg, sr, linux-arm-kernel
My answers are in-line below. BTW bare with me as this is my attempt to get my
feet wet in how to contribute to the linux kernel for my own pleasure and
interest :)
On Wed, Mar 13, 2019 at 03:34:54PM +0300, 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 <armax@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.
>
Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
--fix? If one would like to contribute to fixing the tooling for linting which
of the two would be the right target for such an effort?
> 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.
Funny enough, this is something I adjusted manually :)
> > 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.
Ack.
> >
> > 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.
Ack. In fact, I was thinking this could be one target to fix the logic in
Lindent to do this appropriately.
I have a process question here: to post a change for the only accepted change I
have in this patch should I send out a new patch?
Thanks for the help and the review!
Cheers,
A.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 11:13 ` Armando Miraglia
0 siblings, 0 replies; 46+ messages in thread
From: Armando Miraglia @ 2019-03-14 11:13 UTC (permalink / raw)
To: Dan Carpenter
Cc: devel, sankalpnegi2310, linux-kernel, matthias.bgg,
linux-mediatek, gregkh, neil, sr, linux-arm-kernel
My answers are in-line below. BTW bare with me as this is my attempt to get my
feet wet in how to contribute to the linux kernel for my own pleasure and
interest :)
On Wed, Mar 13, 2019 at 03:34:54PM +0300, 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 <armax@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.
>
Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
--fix? If one would like to contribute to fixing the tooling for linting which
of the two would be the right target for such an effort?
> 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.
Funny enough, this is something I adjusted manually :)
> > 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.
Ack.
> >
> > 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.
Ack. In fact, I was thinking this could be one target to fix the logic in
Lindent to do this appropriately.
I have a process question here: to post a change for the only accepted change I
have in this patch should I send out a new patch?
Thanks for the help and the review!
Cheers,
A.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 11:13 ` Armando Miraglia
(?)
@ 2019-03-14 11:27 ` Dan Carpenter
-1 siblings, 0 replies; 46+ messages in thread
From: Dan Carpenter @ 2019-03-14 11:27 UTC (permalink / raw)
To: Armando Miraglia, Jean Delvare
Cc: neil, devel, gregkh, linux-kernel, linux-mediatek,
sankalpnegi2310, matthias.bgg, sr, linux-arm-kernel
On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote:
> My answers are in-line below. BTW bare with me as this is my attempt to get my
> feet wet in how to contribute to the linux kernel for my own pleasure and
> interest :)
>
No problem at all.
> Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> --fix? If one would like to contribute to fixing the tooling for linting which
> of the two would be the right target for such an effort?
>
I've added Jean to the CC list because he worked on Lindent a few
years ago. I really think we should just delete Lindent. I haven't
used the checkpatch.pl --fix option but Joe Perches has good style.
> > > 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.
>
> Funny enough, this is something I adjusted manually :)
>
:) Good.
> I have a process question here: to post a change for the only accepted change I
> have in this patch should I send out a new patch?
>
Yeah. If you want. Google for how to send a v2 patch.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 11:27 ` Dan Carpenter
0 siblings, 0 replies; 46+ messages in thread
From: Dan Carpenter @ 2019-03-14 11:27 UTC (permalink / raw)
To: Armando Miraglia, Jean Delvare
Cc: devel, sankalpnegi2310, linux-kernel, matthias.bgg,
linux-mediatek, gregkh, neil, sr, linux-arm-kernel
On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote:
> My answers are in-line below. BTW bare with me as this is my attempt to get my
> feet wet in how to contribute to the linux kernel for my own pleasure and
> interest :)
>
No problem at all.
> Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> --fix? If one would like to contribute to fixing the tooling for linting which
> of the two would be the right target for such an effort?
>
I've added Jean to the CC list because he worked on Lindent a few
years ago. I really think we should just delete Lindent. I haven't
used the checkpatch.pl --fix option but Joe Perches has good style.
> > > 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.
>
> Funny enough, this is something I adjusted manually :)
>
:) Good.
> I have a process question here: to post a change for the only accepted change I
> have in this patch should I send out a new patch?
>
Yeah. If you want. Google for how to send a v2 patch.
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 11:27 ` Dan Carpenter
0 siblings, 0 replies; 46+ messages in thread
From: Dan Carpenter @ 2019-03-14 11:27 UTC (permalink / raw)
To: Armando Miraglia, Jean Delvare
Cc: devel, sankalpnegi2310, linux-kernel, matthias.bgg,
linux-mediatek, gregkh, neil, sr, linux-arm-kernel
On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote:
> My answers are in-line below. BTW bare with me as this is my attempt to get my
> feet wet in how to contribute to the linux kernel for my own pleasure and
> interest :)
>
No problem at all.
> Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> --fix? If one would like to contribute to fixing the tooling for linting which
> of the two would be the right target for such an effort?
>
I've added Jean to the CC list because he worked on Lindent a few
years ago. I really think we should just delete Lindent. I haven't
used the checkpatch.pl --fix option but Joe Perches has good style.
> > > 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.
>
> Funny enough, this is something I adjusted manually :)
>
:) Good.
> I have a process question here: to post a change for the only accepted change I
> have in this patch should I send out a new patch?
>
Yeah. If you want. Google for how to send a v2 patch.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 11:27 ` Dan Carpenter
@ 2019-03-14 14:07 ` Jean Delvare
-1 siblings, 0 replies; 46+ messages in thread
From: Jean Delvare @ 2019-03-14 14:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: Armando Miraglia, neil, devel, gregkh, linux-kernel,
linux-mediatek, sankalpnegi2310, matthias.bgg, sr,
linux-arm-kernel
Hi Dan,
On Thu, 14 Mar 2019 14:27:32 +0300, Dan Carpenter wrote:
> On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote:
> > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> > --fix? If one would like to contribute to fixing the tooling for linting which
> > of the two would be the right target for such an effort?
>
> I've added Jean to the CC list because he worked on Lindent a few
> years ago. I really think we should just delete Lindent. I haven't
> used the checkpatch.pl --fix option but Joe Perches has good style.
I merely fixed obvious but minor issues in Lindent as I stumbled upon
them the one time in my life I used that script. That was years ago.
I'm not using it on a regular basis. My principle is that if a script
is present in the kernel tree then it can and should be maintained. If
it is deemed not worth the maintenance effort then it should be
deleted. I don't care either way.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 14:07 ` Jean Delvare
0 siblings, 0 replies; 46+ messages in thread
From: Jean Delvare @ 2019-03-14 14:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: devel, gregkh, linux-kernel, neil, linux-mediatek,
sankalpnegi2310, Armando Miraglia, sr, linux-arm-kernel,
matthias.bgg
Hi Dan,
On Thu, 14 Mar 2019 14:27:32 +0300, Dan Carpenter wrote:
> On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote:
> > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> > --fix? If one would like to contribute to fixing the tooling for linting which
> > of the two would be the right target for such an effort?
>
> I've added Jean to the CC list because he worked on Lindent a few
> years ago. I really think we should just delete Lindent. I haven't
> used the checkpatch.pl --fix option but Joe Perches has good style.
I merely fixed obvious but minor issues in Lindent as I stumbled upon
them the one time in my life I used that script. That was years ago.
I'm not using it on a regular basis. My principle is that if a script
is present in the kernel tree then it can and should be maintained. If
it is deemed not worth the maintenance effort then it should be
deleted. I don't care either way.
--
Jean Delvare
SUSE L3 Support
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 14:07 ` Jean Delvare
@ 2019-03-14 20:50 ` Joe Perches
-1 siblings, 0 replies; 46+ messages in thread
From: Joe Perches @ 2019-03-14 20:50 UTC (permalink / raw)
To: Jean Delvare, Dan Carpenter
Cc: Armando Miraglia, neil, devel, gregkh, linux-kernel,
linux-mediatek, sankalpnegi2310, matthias.bgg, sr,
linux-arm-kernel
On Thu, 2019-03-14 at 15:07 +0100, Jean Delvare wrote:
> My principle is that if a script
> is present in the kernel tree then it can and should be maintained. If
> it is deemed not worth the maintenance effort then it should be
> deleted.
I've suggested deleting Lindent in the past.
https://lkml.org/lkml/2013/2/11/390
Also there is now the clang-format tool that
does an OK job of reflowing source to something
approximating the typical kernel style.
See: Documentation/process/clang-format.rst
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 20:50 ` Joe Perches
0 siblings, 0 replies; 46+ messages in thread
From: Joe Perches @ 2019-03-14 20:50 UTC (permalink / raw)
To: Jean Delvare, Dan Carpenter
Cc: devel, gregkh, linux-kernel, neil, linux-mediatek,
sankalpnegi2310, Armando Miraglia, sr, linux-arm-kernel,
matthias.bgg
On Thu, 2019-03-14 at 15:07 +0100, Jean Delvare wrote:
> My principle is that if a script
> is present in the kernel tree then it can and should be maintained. If
> it is deemed not worth the maintenance effort then it should be
> deleted.
I've suggested deleting Lindent in the past.
https://lkml.org/lkml/2013/2/11/390
Also there is now the clang-format tool that
does an OK job of reflowing source to something
approximating the typical kernel style.
See: Documentation/process/clang-format.rst
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 11:13 ` Armando Miraglia
@ 2019-03-14 11:36 ` Stefan Roese
-1 siblings, 0 replies; 46+ messages in thread
From: Stefan Roese @ 2019-03-14 11:36 UTC (permalink / raw)
To: Armando Miraglia, Dan Carpenter
Cc: neil, devel, gregkh, linux-kernel, linux-mediatek,
sankalpnegi2310, matthias.bgg, linux-arm-kernel
Hi Armando,
On 14.03.19 12:13, Armando Miraglia wrote:
> My answers are in-line below. BTW bare with me as this is my attempt to get my
> feet wet in how to contribute to the linux kernel for my own pleasure and
> interest :)
>
> On Wed, Mar 13, 2019 at 03:34:54PM +0300, 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 <armax@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.
>>
> Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> --fix? If one would like to contribute to fixing the tooling for linting which
> of the two would be the right target for such an effort?
>
>> 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.
>
> Funny enough, this is something I adjusted manually :)
>
>>> 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.
>
> Ack.
>
>>>
>>> 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.
>
> Ack. In fact, I was thinking this could be one target to fix the logic in
> Lindent to do this appropriately.
>
> I have a process question here: to post a change for the only accepted change I
> have in this patch should I send out a new patch?
Would it be possible for you to wait a bit with this minor cleanup?
As I'm preparing a patch to move this driver out of staging right
now. You can definitely follow-up with your cleanup, once this move
is done. Otherwise the move might be delayed even more.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 11:36 ` Stefan Roese
0 siblings, 0 replies; 46+ messages in thread
From: Stefan Roese @ 2019-03-14 11:36 UTC (permalink / raw)
To: Armando Miraglia, Dan Carpenter
Cc: devel, sankalpnegi2310, linux-kernel, matthias.bgg,
linux-mediatek, gregkh, neil, linux-arm-kernel
Hi Armando,
On 14.03.19 12:13, Armando Miraglia wrote:
> My answers are in-line below. BTW bare with me as this is my attempt to get my
> feet wet in how to contribute to the linux kernel for my own pleasure and
> interest :)
>
> On Wed, Mar 13, 2019 at 03:34:54PM +0300, 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 <armax@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.
>>
> Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> --fix? If one would like to contribute to fixing the tooling for linting which
> of the two would be the right target for such an effort?
>
>> 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.
>
> Funny enough, this is something I adjusted manually :)
>
>>> 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.
>
> Ack.
>
>>>
>>> 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.
>
> Ack. In fact, I was thinking this could be one target to fix the logic in
> Lindent to do this appropriately.
>
> I have a process question here: to post a change for the only accepted change I
> have in this patch should I send out a new patch?
Would it be possible for you to wait a bit with this minor cleanup?
As I'm preparing a patch to move this driver out of staging right
now. You can definitely follow-up with your cleanup, once this move
is done. Otherwise the move might be delayed even more.
Thanks,
Stefan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 11:36 ` Stefan Roese
@ 2019-03-14 11:37 ` Armando Miraglia
-1 siblings, 0 replies; 46+ messages in thread
From: Armando Miraglia @ 2019-03-14 11:37 UTC (permalink / raw)
To: Stefan Roese
Cc: Dan Carpenter, Neil Brown, devel, gregkh, linux-kernel,
linux-mediatek, Sankalp Negi, Matthias Brugger, linux-arm-kernel
Absolutely!
Cheers,
A.
On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Armando,
>
> On 14.03.19 12:13, Armando Miraglia wrote:
> > My answers are in-line below. BTW bare with me as this is my attempt to get my
> > feet wet in how to contribute to the linux kernel for my own pleasure and
> > interest :)
> >
> > On Wed, Mar 13, 2019 at 03:34:54PM +0300, 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 <armax@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.
> >>
> > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> > --fix? If one would like to contribute to fixing the tooling for linting which
> > of the two would be the right target for such an effort?
> >
> >> 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.
> >
> > Funny enough, this is something I adjusted manually :)
> >
> >>> 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.
> >
> > Ack.
> >
> >>>
> >>> 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.
> >
> > Ack. In fact, I was thinking this could be one target to fix the logic in
> > Lindent to do this appropriately.
> >
> > I have a process question here: to post a change for the only accepted change I
> > have in this patch should I send out a new patch?
>
> Would it be possible for you to wait a bit with this minor cleanup?
> As I'm preparing a patch to move this driver out of staging right
> now. You can definitely follow-up with your cleanup, once this move
> is done. Otherwise the move might be delayed even more.
>
> Thanks,
> Stefan
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 11:37 ` Armando Miraglia
0 siblings, 0 replies; 46+ messages in thread
From: Armando Miraglia @ 2019-03-14 11:37 UTC (permalink / raw)
To: Stefan Roese
Cc: devel, gregkh, linux-kernel, Matthias Brugger, linux-mediatek,
linux-arm-kernel, Sankalp Negi, Neil Brown, Dan Carpenter
Absolutely!
Cheers,
A.
On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Armando,
>
> On 14.03.19 12:13, Armando Miraglia wrote:
> > My answers are in-line below. BTW bare with me as this is my attempt to get my
> > feet wet in how to contribute to the linux kernel for my own pleasure and
> > interest :)
> >
> > On Wed, Mar 13, 2019 at 03:34:54PM +0300, 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 <armax@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.
> >>
> > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> > --fix? If one would like to contribute to fixing the tooling for linting which
> > of the two would be the right target for such an effort?
> >
> >> 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.
> >
> > Funny enough, this is something I adjusted manually :)
> >
> >>> 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.
> >
> > Ack.
> >
> >>>
> >>> 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.
> >
> > Ack. In fact, I was thinking this could be one target to fix the logic in
> > Lindent to do this appropriately.
> >
> > I have a process question here: to post a change for the only accepted change I
> > have in this patch should I send out a new patch?
>
> Would it be possible for you to wait a bit with this minor cleanup?
> As I'm preparing a patch to move this driver out of staging right
> now. You can definitely follow-up with your cleanup, once this move
> is done. Otherwise the move might be delayed even more.
>
> Thanks,
> Stefan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 11:37 ` Armando Miraglia
(?)
@ 2019-03-14 13:14 ` Matthias Brugger
-1 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-14 13:14 UTC (permalink / raw)
To: Armando Miraglia, Stefan Roese
Cc: Dan Carpenter, Neil Brown, devel, gregkh, linux-kernel,
linux-mediatek, Sankalp Negi, linux-arm-kernel
On 14/03/2019 12:37, Armando Miraglia wrote:
> Absolutely!
Please don't top post :)
>
> Cheers,
> A.
>
> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
[...]
>>
>> Would it be possible for you to wait a bit with this minor cleanup?
>> As I'm preparing a patch to move this driver out of staging right
>> now. You can definitely follow-up with your cleanup, once this move
>> is done. Otherwise the move might be delayed even more.
>>
Hm but shouldn't style issues be a criteria for not accepting a move out of
staging? I think so. You could add Armandos patch in your series or rebase your
series against Greg's tree, once he took the clean-up. Normally Greg is
incredibly fast :)
Regards,
Matthias
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 13:14 ` Matthias Brugger
0 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-14 13:14 UTC (permalink / raw)
To: Armando Miraglia, Stefan Roese
Cc: devel, gregkh, linux-kernel, linux-mediatek, linux-arm-kernel,
Sankalp Negi, Neil Brown, Dan Carpenter
On 14/03/2019 12:37, Armando Miraglia wrote:
> Absolutely!
Please don't top post :)
>
> Cheers,
> A.
>
> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
[...]
>>
>> Would it be possible for you to wait a bit with this minor cleanup?
>> As I'm preparing a patch to move this driver out of staging right
>> now. You can definitely follow-up with your cleanup, once this move
>> is done. Otherwise the move might be delayed even more.
>>
Hm but shouldn't style issues be a criteria for not accepting a move out of
staging? I think so. You could add Armandos patch in your series or rebase your
series against Greg's tree, once he took the clean-up. Normally Greg is
incredibly fast :)
Regards,
Matthias
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 13:14 ` Matthias Brugger
0 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-14 13:14 UTC (permalink / raw)
To: Armando Miraglia, Stefan Roese
Cc: devel, gregkh, linux-kernel, linux-mediatek, linux-arm-kernel,
Sankalp Negi, Neil Brown, Dan Carpenter
On 14/03/2019 12:37, Armando Miraglia wrote:
> Absolutely!
Please don't top post :)
>
> Cheers,
> A.
>
> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
[...]
>>
>> Would it be possible for you to wait a bit with this minor cleanup?
>> As I'm preparing a patch to move this driver out of staging right
>> now. You can definitely follow-up with your cleanup, once this move
>> is done. Otherwise the move might be delayed even more.
>>
Hm but shouldn't style issues be a criteria for not accepting a move out of
staging? I think so. You could add Armandos patch in your series or rebase your
series against Greg's tree, once he took the clean-up. Normally Greg is
incredibly fast :)
Regards,
Matthias
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 13:14 ` Matthias Brugger
@ 2019-03-14 13:24 ` Stefan Roese
-1 siblings, 0 replies; 46+ messages in thread
From: Stefan Roese @ 2019-03-14 13:24 UTC (permalink / raw)
To: Matthias Brugger, Armando Miraglia
Cc: Dan Carpenter, Neil Brown, devel, gregkh, linux-kernel,
linux-mediatek, Sankalp Negi, linux-arm-kernel
On 14.03.19 14:14, Matthias Brugger wrote:
>
>
> On 14/03/2019 12:37, Armando Miraglia wrote:
>> Absolutely!
>
> Please don't top post :)
>
>>
>> Cheers,
>> A.
>>
>> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
> [...]
>>>
>>> Would it be possible for you to wait a bit with this minor cleanup?
>>> As I'm preparing a patch to move this driver out of staging right
>>> now. You can definitely follow-up with your cleanup, once this move
>>> is done. Otherwise the move might be delayed even more.
>>>
>
> Hm but shouldn't style issues be a criteria for not accepting a move out of
> staging?
I would agree, if those style issues where non trivial. In the end we
are talking about one non-optimal identation now.
> I think so. You could add Armandos patch in your series or rebase your
> series against Greg's tree, once he took the clean-up. Normally Greg is
> incredibly fast :)
I should have included the history here to make this more clean. I've
started pulling this driver out of staging a few weeks ago:
https://patchwork.kernel.org/patch/10790537/
...
Here you find Greg's comment, that the style patches should be merged
first before the move out of staging. This is what I worked on after
this first patch series:
https://patchwork.kernel.org/patch/10792455/
...
Now these 9 style issue patches from me have been merged and I would
like to proceed with the driver move.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 13:24 ` Stefan Roese
0 siblings, 0 replies; 46+ messages in thread
From: Stefan Roese @ 2019-03-14 13:24 UTC (permalink / raw)
To: Matthias Brugger, Armando Miraglia
Cc: devel, gregkh, linux-kernel, linux-mediatek, linux-arm-kernel,
Sankalp Negi, Neil Brown, Dan Carpenter
On 14.03.19 14:14, Matthias Brugger wrote:
>
>
> On 14/03/2019 12:37, Armando Miraglia wrote:
>> Absolutely!
>
> Please don't top post :)
>
>>
>> Cheers,
>> A.
>>
>> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
> [...]
>>>
>>> Would it be possible for you to wait a bit with this minor cleanup?
>>> As I'm preparing a patch to move this driver out of staging right
>>> now. You can definitely follow-up with your cleanup, once this move
>>> is done. Otherwise the move might be delayed even more.
>>>
>
> Hm but shouldn't style issues be a criteria for not accepting a move out of
> staging?
I would agree, if those style issues where non trivial. In the end we
are talking about one non-optimal identation now.
> I think so. You could add Armandos patch in your series or rebase your
> series against Greg's tree, once he took the clean-up. Normally Greg is
> incredibly fast :)
I should have included the history here to make this more clean. I've
started pulling this driver out of staging a few weeks ago:
https://patchwork.kernel.org/patch/10790537/
...
Here you find Greg's comment, that the style patches should be merged
first before the move out of staging. This is what I worked on after
this first patch series:
https://patchwork.kernel.org/patch/10792455/
...
Now these 9 style issue patches from me have been merged and I would
like to proceed with the driver move.
Thanks,
Stefan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
2019-03-14 13:24 ` Stefan Roese
@ 2019-03-14 17:01 ` Matthias Brugger
-1 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-14 17:01 UTC (permalink / raw)
To: Stefan Roese, Armando Miraglia
Cc: Dan Carpenter, Neil Brown, devel, gregkh, linux-kernel,
linux-mediatek, Sankalp Negi, linux-arm-kernel
On 14/03/2019 14:24, Stefan Roese wrote:
> On 14.03.19 14:14, Matthias Brugger wrote:
>>
>>
>> On 14/03/2019 12:37, Armando Miraglia wrote:
>>> Absolutely!
>>
>> Please don't top post :)
>>
>>>
>>> Cheers,
>>> A.
>>>
>>> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
>> [...]
>>>>
>>>> Would it be possible for you to wait a bit with this minor cleanup?
>>>> As I'm preparing a patch to move this driver out of staging right
>>>> now. You can definitely follow-up with your cleanup, once this move
>>>> is done. Otherwise the move might be delayed even more.
>>>>
>>
>> Hm but shouldn't style issues be a criteria for not accepting a move out of
>> staging?
>
> I would agree, if those style issues where non trivial. In the end we
> are talking about one non-optimal identation now.>
Fair enough, anyway I'm not the person to decide on that :)
Regards,
Matthias
>> I think so. You could add Armandos patch in your series or rebase your
>> series against Greg's tree, once he took the clean-up. Normally Greg is
>> incredibly fast :)
>
> I should have included the history here to make this more clean. I've
> started pulling this driver out of staging a few weeks ago:
>
> https://patchwork.kernel.org/patch/10790537/
> ...
>
> Here you find Greg's comment, that the style patches should be merged
> first before the move out of staging. This is what I worked on after
> this first patch series:
>
> https://patchwork.kernel.org/patch/10792455/
> ...
>
> Now these 9 style issue patches from me have been merged and I would
> like to proceed with the driver move.
>
> Thanks,
> Stefan
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
@ 2019-03-14 17:01 ` Matthias Brugger
0 siblings, 0 replies; 46+ messages in thread
From: Matthias Brugger @ 2019-03-14 17:01 UTC (permalink / raw)
To: Stefan Roese, Armando Miraglia
Cc: devel, gregkh, linux-kernel, linux-mediatek, linux-arm-kernel,
Sankalp Negi, Neil Brown, Dan Carpenter
On 14/03/2019 14:24, Stefan Roese wrote:
> On 14.03.19 14:14, Matthias Brugger wrote:
>>
>>
>> On 14/03/2019 12:37, Armando Miraglia wrote:
>>> Absolutely!
>>
>> Please don't top post :)
>>
>>>
>>> Cheers,
>>> A.
>>>
>>> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese <sr@denx.de> wrote:
>> [...]
>>>>
>>>> Would it be possible for you to wait a bit with this minor cleanup?
>>>> As I'm preparing a patch to move this driver out of staging right
>>>> now. You can definitely follow-up with your cleanup, once this move
>>>> is done. Otherwise the move might be delayed even more.
>>>>
>>
>> Hm but shouldn't style issues be a criteria for not accepting a move out of
>> staging?
>
> I would agree, if those style issues where non trivial. In the end we
> are talking about one non-optimal identation now.>
Fair enough, anyway I'm not the person to decide on that :)
Regards,
Matthias
>> I think so. You could add Armandos patch in your series or rebase your
>> series against Greg's tree, once he took the clean-up. Normally Greg is
>> incredibly fast :)
>
> I should have included the history here to make this more clean. I've
> started pulling this driver out of staging a few weeks ago:
>
> https://patchwork.kernel.org/patch/10790537/
> ...
>
> Here you find Greg's comment, that the style patches should be merged
> first before the move out of staging. This is what I worked on after
> this first patch series:
>
> https://patchwork.kernel.org/patch/10792455/
> ...
>
> Now these 9 style issue patches from me have been merged and I would
> like to proceed with the driver move.
>
> Thanks,
> Stefan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread