Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
MIME-Version: 1.0
References: <cover.1700449792.git.zhoubinbin@loongson.cn> <fc9b3afaf8826fd437ba91397eb7fa231db2c05c.1700449792.git.zhoubinbin@loongson.cn>
 <87lea4srmx.ffs@tglx>
In-Reply-To: <87lea4srmx.ffs@tglx>
From:   Binbin Zhou <zhoubb.aaron@gmail.com>
Date:   Mon, 11 Dec 2023 08:37:39 +0600
Message-ID: <CAMpQs4+dLk5KG3L0vF4ZBGqbCLKWtCCkQz4wER3yKfsii5GeEA@mail.gmail.com>
Subject: Re: [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map'
 parse
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Binbin Zhou <zhoubinbin@loongson.cn>,
        Huacai Chen <chenhuacai@loongson.cn>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Huacai Chen <chenhuacai@kernel.org>,
        loongson-kernel@lists.loongnix.cn, devicetree@vger.kernel.org,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>,
        linux-mips@vger.kernel.org, lvjianmin@loongson.cn,
        WANG Xuerui <git@xen0n.name>, loongarch@lists.linux.dev,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Dec 8, 2023 at 10:20=E2=80=AFPM Thomas Gleixner <tglx@linutronix.de=
> wrote:
>
> On Mon, Nov 20 2023 at 17:06, Binbin Zhou wrote:
>
> $Subject: s/parse/parsing/
>
> > In keeping with naming standards, 'loongson,parent_int_map' is renamed
> > to 'loongson,parent-int-map'. But for the driver, we need to make sure
> > that both forms can be parsed.
>
> Please keep changelogs in neutral or imperative tone:
>
>   For backwards compatibility it is required to parse the original
>   string too.
>
> Makes it entirely clear what this is about without 'we'. See also:
>
>   https://www.kernel.org/doc/html/latest/process/submitting-patches.rst
>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> >  drivers/irqchip/irq-loongson-liointc.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/i=
rq-loongson-liointc.c
> > index e4b33aed1c97..add2e0a955b8 100644
> > --- a/drivers/irqchip/irq-loongson-liointc.c
> > +++ b/drivers/irqchip/irq-loongson-liointc.c
> > @@ -330,6 +330,7 @@ static int __init liointc_of_init(struct device_nod=
e *node,
> >       bool have_parent =3D FALSE;
> >       int sz, i, index, revision, err =3D 0;
> >       struct resource res;
> > +     const char *prop_name =3D "loongson,parent-int-map";
>
> Please don't glue variables randomly into the declaration section:
>
>   https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#vari=
able-declarations
>
> >       if (!of_device_is_compatible(node, "loongson,liointc-2.0")) {
> >               index =3D 0;
> > @@ -350,8 +351,12 @@ static int __init liointc_of_init(struct device_no=
de *node,
> >       if (!have_parent)
> >               return -ENODEV;
> >
> > +     if (!of_find_property(node, prop_name, &i))
> > +             /* Fallback to 'loongson,parent_int_map'. */
> > +             prop_name =3D "loongson,parent_int_map";
>
> This lacks curly brackets:
>
>   https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#brac=
ket-rules

Hi Thomas:

Thanks for the detailed review.

Rob suggested in the V5 patchset to remove the
'loongson,parent-int-map' renaming operation as an ABI breakage[1]. I
had tried to explain that the driver would be compatible with the
parsing of both naming styles[2], but unfortunately did not get a
response from Rob.
As a result, I removed the renaming-related patches in the V6
patchset, including this one[3].

However, how the 'loongson,parent-int-map' renaming operation is
finally going to be handled needs to be decided together, and if it
remains needed, I will fix the above issue and submit it as part of
the V7 patchset.

[1]: https://lore.kernel.org/all/20231127182836.GA2150516-robh@kernel.org/
[2]: https://lore.kernel.org/all/CAMpQs4LSTV6PgZSuyQx2Nq+87OHxSa=3D-Wz5nbhF=
VsmmvHubQFQ@mail.gmail.com/
[3]: https://lore.kernel.org/all/cover.1701933946.git.zhoubinbin@loongson.c=
n/

Thanks.
Binbin
>
> >       sz =3D of_property_read_variable_u32_array(node,
> > -                                             "loongson,parent_int_map"=
,
> > +                                             prop_name,
> >                                               &parent_int_map[0],
> >                                               LIOINTC_NUM_PARENT,
> >                                               LIOINTC_NUM_PARENT);
>
> Thanks,
>
>         tglx