* [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c @ 2019-03-13 12:24 Armando Miraglia 2019-03-13 12:28 ` Matthias Brugger 2019-03-13 12:34 ` Dan Carpenter 0 siblings, 2 replies; 20+ messages in thread From: Armando Miraglia @ 2019-03-13 12:24 UTC (permalink / raw) To: neil Cc: Armando Miraglia, gregkh, matthias.bgg, sr, sankalpnegi2310, gch981213, devel, linux-arm-kernel, linux-mediatek, linux-kernel 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. 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; }; 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))) 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"}, {}, }; + MODULE_DEVICE_TABLE(of, mt7621_spi_match); 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, + }, .probe = mt7621_spi_probe, .remove = mt7621_spi_remove, }; -- 2.21.0.360.g471c308f928-goog ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 12:24 [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c Armando Miraglia @ 2019-03-13 12:28 ` Matthias Brugger 2019-03-13 12:31 ` Armando Miraglia 2019-03-13 16:34 ` Chuanhong Guo 2019-03-13 12:34 ` Dan Carpenter 1 sibling, 2 replies; 20+ messages in thread From: Matthias Brugger @ 2019-03-13 12:28 UTC (permalink / raw) To: Armando Miraglia, neil Cc: Armando Miraglia, gregkh, sr, sankalpnegi2310, gch981213, devel, linux-arm-kernel, linux-mediatek, linux-kernel On 13/03/2019 13:24, 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. > > Signed-off-by: Armando Miraglia <armax@google.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> Apart from fixing styling issues it would be usefull to see if we can add support for mt7621 to drivers/spi/spi-mt65xx.c Not sure if that is something you want to work on :) Regards, Matthias > --- > 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; > }; > > 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))) > 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"}, > {}, > }; > + > MODULE_DEVICE_TABLE(of, mt7621_spi_match); > > 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, > + }, > .probe = mt7621_spi_probe, > .remove = mt7621_spi_remove, > }; > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 12:28 ` Matthias Brugger @ 2019-03-13 12:31 ` Armando Miraglia 2019-03-13 16:34 ` Chuanhong Guo 1 sibling, 0 replies; 20+ messages in thread From: Armando Miraglia @ 2019-03-13 12:31 UTC (permalink / raw) To: Matthias Brugger Cc: Neil Brown, Armando Miraglia, gregkh, sr, Sankalp Negi, Chuanhong Guo, devel, linux-arm-kernel, linux-mediatek, linux-kernel That might be fun to try :) I should get an mt7621 dev board of sorts though. On Wed, Mar 13, 2019 at 1:28 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > > > > On 13/03/2019 13:24, 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. > > > > Signed-off-by: Armando Miraglia <armax@google.com> > > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > > Apart from fixing styling issues it would be usefull to see if we can add > support for mt7621 to drivers/spi/spi-mt65xx.c > > Not sure if that is something you want to work on :) > > Regards, > Matthias > > > --- > > 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; > > }; > > > > 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))) > > 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"}, > > {}, > > }; > > + > > MODULE_DEVICE_TABLE(of, mt7621_spi_match); > > > > 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, > > + }, > > .probe = mt7621_spi_probe, > > .remove = mt7621_spi_remove, > > }; > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 12:28 ` Matthias Brugger 2019-03-13 12:31 ` Armando Miraglia @ 2019-03-13 16:34 ` Chuanhong Guo 2019-03-13 16:46 ` Matthias Brugger 1 sibling, 1 reply; 20+ messages in thread From: Chuanhong Guo @ 2019-03-13 16:34 UTC (permalink / raw) To: Matthias Brugger Cc: Armando Miraglia, NeilBrown, Armando Miraglia, Greg Kroah-Hartman, Stefan Roese, sankalpnegi2310, devel, linux-arm-kernel, linux-mediatek, linux-kernel Hi! On Wed, Mar 13, 2019 at 8:28 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > > > > On 13/03/2019 13:24, Armando Miraglia wrote: > [...] > Apart from fixing styling issues it would be usefull to see if we can add > support for mt7621 to drivers/spi/spi-mt65xx.c It's impossible. They are completely different IPs. > > Not sure if that is something you want to work on :) > > Regards, > Matthias ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 16:34 ` Chuanhong Guo @ 2019-03-13 16:46 ` Matthias Brugger 2019-03-13 16:54 ` Stefan Roese 0 siblings, 1 reply; 20+ messages in thread From: Matthias Brugger @ 2019-03-13 16:46 UTC (permalink / raw) To: Chuanhong Guo Cc: Armando Miraglia, NeilBrown, Armando Miraglia, Greg Kroah-Hartman, Stefan Roese, sankalpnegi2310, devel, linux-arm-kernel, linux-mediatek, linux-kernel On 13/03/2019 17:34, Chuanhong Guo wrote: > Hi! > On Wed, Mar 13, 2019 at 8:28 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: >> >> >> >> On 13/03/2019 13:24, Armando Miraglia wrote: >> [...] >> Apart from fixing styling issues it would be usefull to see if we can add >> support for mt7621 to drivers/spi/spi-mt65xx.c > It's impossible. They are completely different IPs. Thanks for the info. Do you know the status of the rest of the drivers in staging? Best regards, Matthias ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 16:46 ` Matthias Brugger @ 2019-03-13 16:54 ` Stefan Roese 2019-03-13 22:14 ` NeilBrown 0 siblings, 1 reply; 20+ messages in thread From: Stefan Roese @ 2019-03-13 16:54 UTC (permalink / raw) To: Matthias Brugger, Chuanhong Guo Cc: Armando Miraglia, NeilBrown, Armando Miraglia, Greg Kroah-Hartman, sankalpnegi2310, devel, linux-arm-kernel, linux-mediatek, linux-kernel On 13.03.19 17:46, Matthias Brugger wrote: > > > On 13/03/2019 17:34, Chuanhong Guo wrote: >> Hi! >> On Wed, Mar 13, 2019 at 8:28 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: >>> >>> >>> >>> On 13/03/2019 13:24, Armando Miraglia wrote: >>> [...] >>> Apart from fixing styling issues it would be usefull to see if we can add >>> support for mt7621 to drivers/spi/spi-mt65xx.c >> It's impossible. They are completely different IPs. > > Thanks for the info. Do you know the status of the rest of the drivers in staging? Just to inform you. We are using this SPI driver from staging in one of our customer projects and I will try to move this driver out of staging into drivers/spi very shortly. Thanks, Stefan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 16:54 ` Stefan Roese @ 2019-03-13 22:14 ` NeilBrown 2019-03-14 2:26 ` Chuanhong Guo 0 siblings, 1 reply; 20+ messages in thread From: NeilBrown @ 2019-03-13 22:14 UTC (permalink / raw) To: John Crispin, Stefan Roese, Matthias Brugger, Chuanhong Guo Cc: Armando Miraglia, Armando Miraglia, Greg Kroah-Hartman, sankalpnegi2310, devel, linux-arm-kernel, linux-mediatek, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1322 bytes --] On Wed, Mar 13 2019, Stefan Roese wrote: > On 13.03.19 17:46, Matthias Brugger wrote: >> >> >> On 13/03/2019 17:34, Chuanhong Guo wrote: >>> Hi! >>> On Wed, Mar 13, 2019 at 8:28 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: >>>> >>>> >>>> >>>> On 13/03/2019 13:24, Armando Miraglia wrote: >>>> [...] >>>> Apart from fixing styling issues it would be usefull to see if we can add >>>> support for mt7621 to drivers/spi/spi-mt65xx.c >>> It's impossible. They are completely different IPs. >> >> Thanks for the info. Do you know the status of the rest of the drivers in staging? > > Just to inform you. We are using this SPI driver from staging > in one of our customer projects and I will try to move this > driver out of staging into drivers/spi very shortly. This is good news! I think the driver is ready to move out of staging and would be very happy to see you do it. My only small concern is that this driver was backported to openwrt (4.14 based) and then reverted https://github.com/openwrt/openwrt/commit/749a29f76ca780d8df70a5163d43bbdc6f13ba3f "This breaks some mt7621 devices." Possibly it was backported badly, or possibly there is a problem. John: do you have any more details of the problem other than what is in the commit message? Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 22:14 ` NeilBrown @ 2019-03-14 2:26 ` Chuanhong Guo 2019-03-14 2:36 ` NeilBrown 0 siblings, 1 reply; 20+ messages in thread From: Chuanhong Guo @ 2019-03-14 2:26 UTC (permalink / raw) To: NeilBrown Cc: John Crispin, Stefan Roese, Matthias Brugger, Armando Miraglia, Armando Miraglia, Greg Kroah-Hartman, sankalpnegi2310, devel, linux-arm-kernel, linux-mediatek, linux-kernel Hi! On Thu, Mar 14, 2019 at 6:14 AM NeilBrown <neil@brown.name> wrote: > > [...] > My only small concern is that this driver was backported to openwrt > (4.14 based) and then reverted > > https://github.com/openwrt/openwrt/commit/749a29f76ca780d8df70a5163d43bbdc6f13ba3f > > "This breaks some mt7621 devices." > > Possibly it was backported badly, or possibly there is a problem. Last time I backported the version with broken SPI modes so it got broken SPI CS1 support. There is one mt7621 router using SPI CS1 in OpenWrt and the backported driver broke it. It has been fixed by my two "drop broken spi modes" patches. > > John: do you have any more details of the problem other than what is in > the commit message? > > Thanks, > NeilBrown Regards, Chuanhong Guo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-14 2:26 ` Chuanhong Guo @ 2019-03-14 2:36 ` NeilBrown 0 siblings, 0 replies; 20+ messages in thread From: NeilBrown @ 2019-03-14 2:36 UTC (permalink / raw) To: Chuanhong Guo Cc: John Crispin, Stefan Roese, Matthias Brugger, Armando Miraglia, Armando Miraglia, Greg Kroah-Hartman, sankalpnegi2310, devel, linux-arm-kernel, linux-mediatek, linux-kernel [-- Attachment #1: Type: text/plain, Size: 944 bytes --] On Thu, Mar 14 2019, Chuanhong Guo wrote: > Hi! > On Thu, Mar 14, 2019 at 6:14 AM NeilBrown <neil@brown.name> wrote: >> >> [...] >> My only small concern is that this driver was backported to openwrt >> (4.14 based) and then reverted >> >> https://github.com/openwrt/openwrt/commit/749a29f76ca780d8df70a5163d43bbdc6f13ba3f >> >> "This breaks some mt7621 devices." >> >> Possibly it was backported badly, or possibly there is a problem. > Last time I backported the version with broken SPI modes so it got > broken SPI CS1 support. There is one mt7621 router using SPI CS1 in > OpenWrt and the backported driver broke it. > It has been fixed by my two "drop broken spi modes" patches. Ahh, thanks for the clarification. Good to know all known problems are fixed. Thanks, NeilBrown >> >> John: do you have any more details of the problem other than what is in >> the commit message? >> >> Thanks, >> NeilBrown > > Regards, > Chuanhong Guo [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c 2019-03-13 12:24 [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c Armando Miraglia 2019-03-13 12:28 ` Matthias Brugger @ 2019-03-13 12:34 ` Dan Carpenter 2019-03-13 16:47 ` Matthias Brugger 2019-03-14 11:13 ` Armando Miraglia 1 sibling, 2 replies; 20+ messages in thread From: Dan Carpenter @ 2019-03-13 12:34 UTC (permalink / raw) To: Armando Miraglia Cc: neil, devel, gregkh, linux-kernel, Armando Miraglia, linux-mediatek, sankalpnegi2310, matthias.bgg, 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] 20+ 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 2019-03-14 11:13 ` Armando Miraglia 1 sibling, 0 replies; 20+ 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] 20+ 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 @ 2019-03-14 11:13 ` Armando Miraglia 2019-03-14 11:27 ` Dan Carpenter 2019-03-14 11:36 ` Stefan Roese 1 sibling, 2 replies; 20+ 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] 20+ 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 2019-03-14 14:07 ` Jean Delvare 2019-03-14 11:36 ` Stefan Roese 1 sibling, 1 reply; 20+ 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] 20+ 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 2019-03-14 20:50 ` Joe Perches 0 siblings, 1 reply; 20+ 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] 20+ 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 0 siblings, 0 replies; 20+ 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] 20+ 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 @ 2019-03-14 11:36 ` Stefan Roese 2019-03-14 11:37 ` Armando Miraglia 1 sibling, 1 reply; 20+ 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] 20+ 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 2019-03-14 13:14 ` Matthias Brugger 0 siblings, 1 reply; 20+ 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] 20+ 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 2019-03-14 13:24 ` Stefan Roese 0 siblings, 1 reply; 20+ 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] 20+ 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 2019-03-14 17:01 ` Matthias Brugger 0 siblings, 1 reply; 20+ 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] 20+ 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 0 siblings, 0 replies; 20+ 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] 20+ messages in thread
end of thread, other threads:[~2019-03-14 20:50 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-13 12:24 [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c Armando Miraglia 2019-03-13 12:28 ` Matthias Brugger 2019-03-13 12:31 ` Armando Miraglia 2019-03-13 16:34 ` Chuanhong Guo 2019-03-13 16:46 ` Matthias Brugger 2019-03-13 16:54 ` Stefan Roese 2019-03-13 22:14 ` NeilBrown 2019-03-14 2:26 ` Chuanhong Guo 2019-03-14 2:36 ` NeilBrown 2019-03-13 12:34 ` Dan Carpenter 2019-03-13 16:47 ` Matthias Brugger 2019-03-14 11:13 ` Armando Miraglia 2019-03-14 11:27 ` Dan Carpenter 2019-03-14 14:07 ` Jean Delvare 2019-03-14 20:50 ` Joe Perches 2019-03-14 11:36 ` Stefan Roese 2019-03-14 11:37 ` Armando Miraglia 2019-03-14 13:14 ` Matthias Brugger 2019-03-14 13:24 ` Stefan Roese 2019-03-14 17:01 ` Matthias Brugger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).