RE: [PATCH v1 06/16] clk: starfive: Add JH8100 System clock generator driver

[JeeHeng Sia]

> -----Original Message-----
> From: Emil Renner Berthing <emil.renner.berthing@xxxxxxxxxxxxx>
> Sent: Wednesday, December 13, 2023 8:05 PM
> To: JeeHeng Sia <jeeheng.sia@xxxxxxxxxxxxxxxx>; Emil Renner Berthing <emil.renner.berthing@xxxxxxxxxxxxx>; kernel@xxxxxxxx;
> conor@xxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; paul.walmsley@xxxxxxxxxx; palmer@xxxxxxxxxxx;
> aou@xxxxxxxxxxxxxxxxx; mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx; Hal Feng
> <hal.feng@xxxxxxxxxxxxxxxx>; Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx>
> Cc: linux-riscv@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; Leyfoon Tan
> <leyfoon.tan@xxxxxxxxxxxxxxxx>
> Subject: RE: [PATCH v1 06/16] clk: starfive: Add JH8100 System clock generator driver
> 
> JeeHeng Sia wrote:
> >
> >
> > > -----Original Message-----
> > > From: Emil Renner Berthing <emil.renner.berthing@xxxxxxxxxxxxx>
> > > Sent: Saturday, December 9, 2023 12:25 AM
> > > To: JeeHeng Sia <jeeheng.sia@xxxxxxxxxxxxxxxx>; kernel@xxxxxxxx; conor@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> > > krzysztof.kozlowski+dt@xxxxxxxxxx; paul.walmsley@xxxxxxxxxx; palmer@xxxxxxxxxxx; aou@xxxxxxxxxxxxxxxxx;
> > > mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx; emil.renner.berthing@xxxxxxxxxxxxx; Hal Feng
> > > <hal.feng@xxxxxxxxxxxxxxxx>; Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx>
> > > Cc: linux-riscv@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; Leyfoon
> Tan
> > > <leyfoon.tan@xxxxxxxxxxxxxxxx>
> > > Subject: Re: [PATCH v1 06/16] clk: starfive: Add JH8100 System clock generator driver
> > >
> > > Sia Jee Heng wrote:
> > > > Add support for JH8100 System clock generator.
> > > >
> > > > Signed-off-by: Sia Jee Heng <jeeheng.sia@xxxxxxxxxxxxxxxx>
> > > > Reviewed-by: Ley Foon Tan <leyfoon.tan@xxxxxxxxxxxxxxxx>
> > > > ---
> > > >  MAINTAINERS                                   |   8 +
> > > >  drivers/clk/starfive/Kconfig                  |   9 +
> > > >  drivers/clk/starfive/Makefile                 |   1 +
> > > >  drivers/clk/starfive/clk-starfive-common.h    |   9 +-
> > > >  drivers/clk/starfive/jh8100/Makefile          |   3 +
> > > >  .../clk/starfive/jh8100/clk-starfive-jh8100.h |  11 +
> > > >  drivers/clk/starfive/jh8100/clk-sys.c         | 455 ++++++++++++++++++
> > > >  7 files changed, 495 insertions(+), 1 deletion(-)
> > > >  create mode 100644 drivers/clk/starfive/jh8100/Makefile
> > > >  create mode 100644 drivers/clk/starfive/jh8100/clk-starfive-jh8100.h
> > > >  create mode 100644 drivers/clk/starfive/jh8100/clk-sys.c
> > > >
> [...]
> > > > diff --git a/drivers/clk/starfive/jh8100/Makefile b/drivers/clk/starfive/jh8100/Makefile
> > > > new file mode 100644
> > > > index 000000000000..af6a09e220d3
> > > > --- /dev/null
> > > > +++ b/drivers/clk/starfive/jh8100/Makefile
> > > > @@ -0,0 +1,3 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# StarFive JH8100 Clock
> > > > +obj-$(CONFIG_CLK_STARFIVE_JH8100_SYS)		+= clk-sys.o
> > >
> > > This will name the module clk-sys, which is way too generic. Please name this
> > > clk-starfive-jh8100-sys similar to the JH7110 drivers.
> > Just realized that I haven't reply to this comment.
> > I can't give it a longer name otherwise compiler will throw warning.
> > That’s why ends up to use a shorter name and keep it under jh8100 folder.
> 
> I'm sorry, how does that make any sense? If the compiler can compile
> 
>   drivers/clk/starfive/clk-starfive-jh7110-sys.c
> 
> just fine, then why would it have trouble with
> 
>   drivers/clk/starfive/clk-starfive-jh8100-sys.c
Based on the experiment conducted at the early stage, the warning message is coming
from the name of the auxiliary device (reset in this case). We try to map the name of
the reset device with the file's name, and at that moment, the file name is longer than
32 characters in length. In your given example, 'clk-starfive-jh8100-sys' is within the
32-character limit, so it should be okay. Given that the trend is not to use subfolders to
differentiate the platform clock driver, I am okay with removing the subfolder and
following the naming convention from the JH71xx's file name.
> 
> /Emil