Re: [PATCH v13 01/13] irqchip/sifive-plic: Convert PLIC driver into a platform driver

From: Anup Patel
Date: Thu Feb 22 2024 - 04:26:04 EST


On Tue, Feb 20, 2024 at 3:39 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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