[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK9=C2V_SVWTuV_Yap_=X5RwgctUs7eWHGXSp8KFUz3bWNf9JQ@mail.gmail.com>
Date: Thu, 22 Feb 2024 14:55:41 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <paul.walmsley@...ive.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Frank Rowand <frowand.list@...il.com>,
Conor Dooley <conor+dt@...nel.org>, Marc Zyngier <maz@...nel.org>, Björn Töpel <bjorn@...nel.org>,
Atish Patra <atishp@...shpatra.org>, Andrew Jones <ajones@...tanamicro.com>,
Sunil V L <sunilvl@...tanamicro.com>, Saravana Kannan <saravanak@...gle.com>,
Anup Patel <anup@...infault.org>, linux-riscv@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v13 01/13] irqchip/sifive-plic: Convert PLIC driver into a
platform driver
On Tue, Feb 20, 2024 at 3:39 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Tue, Feb 20 2024 at 11:37, Anup Patel wrote:
> > The PLIC driver does not require very early initialization so let
> > us convert it into a platform driver.
>
> s/let us convert/convert/
>
> Please us passive voice and imperative mood all over the changelogs. No
> we/us, let....
Okay, I will update.
>
> > As part of the conversion, the PLIC probing undergoes the following
> > changes:
> > 1. Use dev_info(), dev_err() and dev_warn() instead of pr_info(),
> > pr_err() and pr_warn()
> > 2. Use devm_xyz() APIs wherever applicable
> > 3. PLIC is now probed after CPUs are brought-up so we have to
> > setup cpuhp state after context handler of all online CPUs
> > are initialized otherwise we see crash on multi-socket systems
>
> This patch is really doing too many things at once, which makes it hard
> to review. Can you split this into digestable pieces please?
Sure, I will split this into smaller granular patches.
>
> > if (unlikely(err))
> > - pr_warn_ratelimited("can't find mapping for hwirq %lu\n",
> > + dev_warn_ratelimited(handler->priv->dev,
> > + "can't find mapping for hwirq %lu\n",
> > hwirq);
>
> Nit. Please use brackets around the condition. See:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-rules
>
> for reasoning.
Okay, I will update.
Regards,
Anup
Powered by blists - more mailing lists