On Fri, Dec 8, 2023 at 10:20 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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@xxxxxxxxxxx> > > Acked-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> > > Reviewed-by: Huacai Chen <chenhuacai@xxxxxxxxxxx> > > --- > > 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/irq-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_node *node, > > bool have_parent = FALSE; > > int sz, i, index, revision, err = 0; > > struct resource res; > > + const char *prop_name = "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#variable-declarations > > > if (!of_device_is_compatible(node, "loongson,liointc-2.0")) { > > index = 0; > > @@ -350,8 +351,12 @@ static int __init liointc_of_init(struct device_node *node, > > if (!have_parent) > > return -ENODEV; > > > > + if (!of_find_property(node, prop_name, &i)) > > + /* Fallback to 'loongson,parent_int_map'. */ > > + prop_name = "loongson,parent_int_map"; > > This lacks curly brackets: > > https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-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@xxxxxxxxxx/ [2]: https://lore.kernel.org/all/CAMpQs4LSTV6PgZSuyQx2Nq+87OHxSa=-Wz5nbhFVsmmvHubQFQ@xxxxxxxxxxxxxx/ [3]: https://lore.kernel.org/all/cover.1701933946.git.zhoubinbin@xxxxxxxxxxx/ Thanks. Binbin > > > sz = 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
