From: Marc Zyngier <maz@kernel.org>
To: Daniel Baluta <daniel.baluta@gmail.com>
Cc: jason@lakedaemon.net, Anson Huang <Anson.Huang@nxp.com>,
Fabio Estevam <festevam@gmail.com>,
Sascha Hauer <s.hauer@pengutronix.de>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
dl-linux-imx <Linux-imx@nxp.com>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
tglx@linutronix.de, Shawn Guo <shawnguo@kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] irqchip: irq-imx-gpcv2: Remove unnecessary blank lines
Date: Sun, 22 Mar 2020 19:35:56 +0000 [thread overview]
Message-ID: <a690ecc9169f0594e22b4eae0a056d89@kernel.org> (raw)
In-Reply-To: <CAEnQRZCcmaU91Ep5kVesaGPsrvujq5mznk2SZccmZG9rbSCG0w@mail.gmail.com>
Daniel,
On 2020-03-22 19:08, Daniel Baluta wrote:
> On Sat, Mar 21, 2020 at 5:22 PM Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Tue, 17 Mar 2020 04:56:41 +0000,
>> Anson Huang <Anson.Huang@nxp.com> wrote:
>> >
>> > Remove unnecessary blank lines for cleanup.
>> >
>> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>> > ---
>> > drivers/irqchip/irq-imx-gpcv2.c | 2 --
>> > 1 file changed, 2 deletions(-)
>> >
>> > diff --git a/drivers/irqchip/irq-imx-gpcv2.c b/drivers/irqchip/irq-imx-gpcv2.c
>> > index 4f74c15..4f11b9b 100644
>> > --- a/drivers/irqchip/irq-imx-gpcv2.c
>> > +++ b/drivers/irqchip/irq-imx-gpcv2.c
>> > @@ -17,7 +17,6 @@
>> > #define GPC_IMR1_CORE2 0x1c0
>> > #define GPC_IMR1_CORE3 0x1d0
>> >
>> > -
>> > struct gpcv2_irqchip_data {
>> > struct raw_spinlock rlock;
>> > void __iomem *gpc_base;
>> > @@ -287,6 +286,5 @@ static int __init imx_gpcv2_irqchip_init(struct device_node *node,
>> > of_node_clear_flag(node, OF_POPULATED);
>> > return 0;
>> > }
>> > -
>> > IRQCHIP_DECLARE(imx_gpcv2_imx7d, "fsl,imx7d-gpc", imx_gpcv2_irqchip_init);
>> > IRQCHIP_DECLARE(imx_gpcv2_imx8mq, "fsl,imx8mq-gpc", imx_gpcv2_irqchip_init);
>>
>> I honestly don't think this deserves a patch. Next time you work on
>> this driver, add the cleanups to it. But on its own, it's only churn.
>
> While you are technically right, it's only churn I think we need this
> for code consistency and cleanup.
Get real. We really don't. Two blank lines do not lead to a
misinterpretation of the code, do not get in the way of normal
maintenance, do not lead to *any* practical issue.
What's next? Cc stable?
> Even if we fix this when we work on the driver we still need
> to make the cleanup in a separate patch.
Neither. As well as removing blank lines, you could also remove the
dead code in this driver. That would be a good cleanup. You could
also have a look at what feels like a potential deadlock in the
mask/unmask callbacks. That'd be a good thing to do too.
Certainly more useful than just dropping two blank lines.
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Anson Huang <Anson.Huang@nxp.com>,
tglx@linutronix.de, jason@lakedaemon.net,
Shawn Guo <shawnguo@kernel.org>,
Sascha Hauer <s.hauer@pengutronix.de>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Fabio Estevam <festevam@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
dl-linux-imx <Linux-imx@nxp.com>
Subject: Re: [PATCH] irqchip: irq-imx-gpcv2: Remove unnecessary blank lines
Date: Sun, 22 Mar 2020 19:35:56 +0000 [thread overview]
Message-ID: <a690ecc9169f0594e22b4eae0a056d89@kernel.org> (raw)
In-Reply-To: <CAEnQRZCcmaU91Ep5kVesaGPsrvujq5mznk2SZccmZG9rbSCG0w@mail.gmail.com>
Daniel,
On 2020-03-22 19:08, Daniel Baluta wrote:
> On Sat, Mar 21, 2020 at 5:22 PM Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Tue, 17 Mar 2020 04:56:41 +0000,
>> Anson Huang <Anson.Huang@nxp.com> wrote:
>> >
>> > Remove unnecessary blank lines for cleanup.
>> >
>> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>> > ---
>> > drivers/irqchip/irq-imx-gpcv2.c | 2 --
>> > 1 file changed, 2 deletions(-)
>> >
>> > diff --git a/drivers/irqchip/irq-imx-gpcv2.c b/drivers/irqchip/irq-imx-gpcv2.c
>> > index 4f74c15..4f11b9b 100644
>> > --- a/drivers/irqchip/irq-imx-gpcv2.c
>> > +++ b/drivers/irqchip/irq-imx-gpcv2.c
>> > @@ -17,7 +17,6 @@
>> > #define GPC_IMR1_CORE2 0x1c0
>> > #define GPC_IMR1_CORE3 0x1d0
>> >
>> > -
>> > struct gpcv2_irqchip_data {
>> > struct raw_spinlock rlock;
>> > void __iomem *gpc_base;
>> > @@ -287,6 +286,5 @@ static int __init imx_gpcv2_irqchip_init(struct device_node *node,
>> > of_node_clear_flag(node, OF_POPULATED);
>> > return 0;
>> > }
>> > -
>> > IRQCHIP_DECLARE(imx_gpcv2_imx7d, "fsl,imx7d-gpc", imx_gpcv2_irqchip_init);
>> > IRQCHIP_DECLARE(imx_gpcv2_imx8mq, "fsl,imx8mq-gpc", imx_gpcv2_irqchip_init);
>>
>> I honestly don't think this deserves a patch. Next time you work on
>> this driver, add the cleanups to it. But on its own, it's only churn.
>
> While you are technically right, it's only churn I think we need this
> for code consistency and cleanup.
Get real. We really don't. Two blank lines do not lead to a
misinterpretation of the code, do not get in the way of normal
maintenance, do not lead to *any* practical issue.
What's next? Cc stable?
> Even if we fix this when we work on the driver we still need
> to make the cleanup in a separate patch.
Neither. As well as removing blank lines, you could also remove the
dead code in this driver. That would be a good cleanup. You could
also have a look at what feels like a potential deadlock in the
mask/unmask callbacks. That'd be a good thing to do too.
Certainly more useful than just dropping two blank lines.
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2020-03-22 19:36 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-17 4:56 [PATCH] irqchip: irq-imx-gpcv2: Remove unnecessary blank lines Anson Huang
2020-03-17 4:56 ` Anson Huang
2020-03-21 15:21 ` Marc Zyngier
2020-03-21 15:21 ` Marc Zyngier
2020-03-22 19:08 ` Daniel Baluta
2020-03-22 19:08 ` Daniel Baluta
2020-03-22 19:35 ` Marc Zyngier [this message]
2020-03-22 19:35 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a690ecc9169f0594e22b4eae0a056d89@kernel.org \
--to=maz@kernel.org \
--cc=Anson.Huang@nxp.com \
--cc=Linux-imx@nxp.com \
--cc=daniel.baluta@gmail.com \
--cc=festevam@gmail.com \
--cc=jason@lakedaemon.net \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.