[PATCH 00/15] Ambarella S6LM SoC bring-up

[PATCH 00/15] Ambarella S6LM SoC bring-up

[Li Chen]

This series brings up initial support for the Ambarella S6LM
SoC.

The following features are supported in this initial port:

- UART with console support
- Pinctrl with GPIO controller
- Nand flash controller
- Devicetree

Li Chen (15):
  debugfs: allow to use regmap for print regs
  dt-bindings: vendor-prefixes: add Ambarella prefix
  dt-bindings: arm: ambarella: Add binding for Ambarella ARM platforms
  dt-bindings: arm: add support for Ambarella SoC
  arm64: Kconfig: Introduce CONFIG_ARCH_AMBARELLA
  soc: add Ambarella driver
  dt-bindings: clock: Add Ambarella clock bindings
  clk: add support for Ambarella clocks
  dt-bindings: serial: add support for Ambarella
  serial: ambarella: add support for Ambarella uart_port
  dt-bindings: mtd: Add binding for Ambarella
  mtd: nand: add Ambarella nand support
  dt-bindings: pinctrl: add support for Ambarella
  pinctrl: Add pinctrl/GPIO for Ambarella SoCs
  arm64: dts: ambarella: introduce Ambarella s6lm SoC

 .../devicetree/bindings/arm/ambarella.yaml    |   22 +
 .../arm/ambarella/ambarella,cpuid.yaml        |   24 +
 .../bindings/arm/ambarella/ambarella,rct.yaml |   24 +
 .../arm/ambarella/ambarella,scratchpad.yaml   |   24 +
 .../bindings/arm/ambarella/ambarella.yaml     |   22 +
 .../clock/ambarella,composite-clock.yaml      |   52 +
 .../bindings/clock/ambarella,pll-clock.yaml   |   59 +
 .../bindings/mtd/ambarella,nand.yaml          |   77 +
 .../bindings/pinctrl/ambarella,pinctrl.yaml   |  160 ++
 .../bindings/serial/ambarella_uart.yaml       |   57 +
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 Documentation/filesystems/debugfs.rst         |    2 +
 MAINTAINERS                                   |   29 +
 arch/arm64/Kconfig.platforms                  |    9 +
 .../boot/dts/ambarella/ambarella-s6lm.dtsi    |  332 ++++
 .../boot/dts/ambarella/s6lm_pineapple.dts     |   29 +
 drivers/clk/Makefile                          |    1 +
 drivers/clk/ambarella/Makefile                |    5 +
 drivers/clk/ambarella/clk-composite.c         |  293 +++
 drivers/clk/ambarella/clk-pll-common.c        |  308 ++++
 drivers/clk/ambarella/clk-pll-common.h        |   96 +
 drivers/clk/ambarella/clk-pll-normal.c        |  328 ++++
 drivers/mtd/nand/raw/Kconfig                  |    8 +
 drivers/mtd/nand/raw/Makefile                 |    1 +
 drivers/mtd/nand/raw/ambarella_combo_nand.c   | 1519 ++++++++++++++++
 drivers/mtd/nand/raw/ambarella_combo_nand.h   |  370 ++++
 drivers/mtd/nand/raw/nand_ids.c               |    4 +
 drivers/pinctrl/Kconfig                       |    6 +
 drivers/pinctrl/Makefile                      |    1 +
 drivers/pinctrl/pinctrl-ambarella.c           | 1357 ++++++++++++++
 drivers/soc/Makefile                          |    1 +
 drivers/soc/ambarella/Makefile                |    3 +
 drivers/soc/ambarella/soc.c                   |  136 ++
 drivers/tty/serial/Kconfig                    |   16 +
 drivers/tty/serial/Makefile                   |    1 +
 drivers/tty/serial/ambarella_uart.c           | 1581 +++++++++++++++++
 drivers/tty/serial/ambarella_uart.h           |  120 ++
 fs/debugfs/file.c                             |   43 +-
 include/linux/debugfs.h                       |   11 +
 include/soc/ambarella/misc.h                  |   17 +
 40 files changed, 7149 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/ambarella.yaml
 create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
 create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
 create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
 create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/ambarella,composite-clock.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/ambarella,pll-clock.yaml
 create mode 100644 Documentation/devicetree/bindings/mtd/ambarella,nand.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ambarella,pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/serial/ambarella_uart.yaml
 create mode 100644 arch/arm64/boot/dts/ambarella/ambarella-s6lm.dtsi
 create mode 100644 arch/arm64/boot/dts/ambarella/s6lm_pineapple.dts
 create mode 100644 drivers/clk/ambarella/Makefile
 create mode 100644 drivers/clk/ambarella/clk-composite.c
 create mode 100644 drivers/clk/ambarella/clk-pll-common.c
 create mode 100644 drivers/clk/ambarella/clk-pll-common.h
 create mode 100644 drivers/clk/ambarella/clk-pll-normal.c
 create mode 100644 drivers/mtd/nand/raw/ambarella_combo_nand.c
 create mode 100644 drivers/mtd/nand/raw/ambarella_combo_nand.h
 create mode 100644 drivers/pinctrl/pinctrl-ambarella.c
 create mode 100644 drivers/soc/ambarella/Makefile
 create mode 100644 drivers/soc/ambarella/soc.c
 create mode 100644 drivers/tty/serial/ambarella_uart.c
 create mode 100644 drivers/tty/serial/ambarella_uart.h
 create mode 100644 include/soc/ambarella/misc.h

-- 
2.34.1

Re: [PATCH 00/15] Ambarella S6LM SoC bring-up

[Arnd Bergmann]

On Mon, Jan 23, 2023, at 08:32, Li Chen wrote:
> This series brings up initial support for the Ambarella S6LM
> SoC.
>
> The following features are supported in this initial port:
>
> - UART with console support
> - Pinctrl with GPIO controller
> - Nand flash controller
> - Devicetree

I seem to only have part of the series, please add both me and
the linux-arm-kernel mailing list to each part of the initial
submission.

It's possible that some patches were already Cc'd to
linux-arm-kernel but did not make it through because the Cc list
was too long (it has to fit within 1024 characters for many lists).
I think you too the Cc list from get_maintainers.pl, but when
sending new drivers this does not work well because it picks
up everyone that recently touched the Makefile/Kconfig.

     Arnd

Re: [PATCH 01/15] debugfs: allow to use regmap for print regs

[Greg Kroah-Hartman]

On Mon, Jan 23, 2023 at 03:32:16PM +0800, Li Chen wrote:
> Currently, debugfs_regset32 only contains void __iomem *base,
> and it is not friendly to regmap user.
> 
> Let's add regmap to debugfs_regset32, and add regmap
> support to debugfs_print_reg32.
> 
> Signed-off-by: Li Chen <me@linux.beauty>
> Change-Id: I8ef015ed0906a4ad85b7592f771dcf64c23f7832

No change-id please.

> ---
>  Documentation/filesystems/debugfs.rst |  2 ++
>  fs/debugfs/file.c                     | 43 ++++++++++++++++++++++++++-
>  include/linux/debugfs.h               | 11 +++++++
>  3 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/filesystems/debugfs.rst b/Documentation/filesystems/debugfs.rst
> index dc35da8b8792..b2c76ac3a333 100644
> --- a/Documentation/filesystems/debugfs.rst
> +++ b/Documentation/filesystems/debugfs.rst
> @@ -178,6 +178,8 @@ file::
>  
>      void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
>  			 int nregs, void __iomem *base, char *prefix);
> +    void debugfs_print_regmap_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
> +			 int nregs, struct regmap *regmap*, char *prefix);

One too many "*" characters on that last line, right?

>  
>  The "base" argument may be 0, but you may want to build the reg32 array
>  using __stringify, and a number of register names (macros) are actually
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index b54f470e0d03..2fb792843b30 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -1137,14 +1137,55 @@ void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
>  }
>  EXPORT_SYMBOL_GPL(debugfs_print_regs32);
>  
> +/**
> + * debugfs_print_regmap_regs32 - use seq_print to describe a set of registers
> + * @s: the seq_file structure being used to generate output
> + * @regs: an array if struct debugfs_reg32 structures
> + * @nregs: the length of the above array
> + * @regmap: regmap to be used in reading the registers
> + * @prefix: a string to be prefixed to every output line
> + *
> + * This function outputs a text block describing the current values of
> + * some 32-bit hardware registers. It is meant to be used within debugfs
> + * files based on seq_file that need to show registers, intermixed with other
> + * information. The prefix argument may be used to specify a leading string,
> + * because some peripherals have several blocks of identical registers,
> + * for example configuration of dma channels
> + */
> +void debugfs_print_regmap_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
> +			  int nregs, struct regmap *regmap, char *prefix)
> +{
> +	int i;
> +	u32 val;
> +
> +	for (i = 0; i < nregs; i++, regs++) {
> +		if (prefix)
> +			seq_printf(s, "%s", prefix);
> +		regmap_read(regmap, regs->offset, &val);
> +		seq_printf(s, "%s = 0x%08x\n", regs->name, val);
> +		if (seq_has_overflowed(s))
> +			break;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(debugfs_print_regmap_regs32);
> +
>  static int debugfs_regset32_show(struct seq_file *s, void *data)
>  {
>  	struct debugfs_regset32 *regset = s->private;
>  
> +	void __iomem *base = regset->base;
> +	struct regmap *regmap = regset->regmap;

Why the extra blank line?  Did you run checkpatch?

And it's generally not considered a good idea to dereference a pointer
_before_ it is checked.  It will not crash, but static checkers will
have a field day with it.


> +
> +	if ((regmap && base) || (!regmap && !base))
> +		return -EINVAL;
> +
>  	if (regset->dev)
>  		pm_runtime_get_sync(regset->dev);
>  
> -	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
> +	if (base)
> +		debugfs_print_regs32(s, regset->regs, regset->nregs, base, "");
> +	else
> +		debugfs_print_regmap_regs32(s, regset->regs, regset->nregs, regmap, "");
>  
>  	if (regset->dev)
>  		pm_runtime_put(regset->dev);
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index ea2d919fd9c7..87dfea6a25a0 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -17,6 +17,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/compiler.h>
> +#include <linux/regmap.h>

No need to include this here, just provide a prototype for "struct
regmap" and all will be fine.

thanks,

greg k-h

Re: [PATCH 01/15] debugfs: allow to use regmap for print regs

[Li Chen]

Hi Greg,

On Mon, 23 Jan 2023 19:52:45 +0800,
Greg Kroah-Hartman wrote:
>
> On Mon, Jan 23, 2023 at 03:32:16PM +0800, Li Chen wrote:
> > Currently, debugfs_regset32 only contains void __iomem *base,
> > and it is not friendly to regmap user.
> >
> > Let's add regmap to debugfs_regset32, and add regmap
> > support to debugfs_print_reg32.
> >
> > Signed-off-by: Li Chen <me@linux.beauty>
> > Change-Id: I8ef015ed0906a4ad85b7592f771dcf64c23f7832
>
> No change-id please.

Sorry, my bad, will remove it in v2.

> > ---
> >  Documentation/filesystems/debugfs.rst |  2 ++
> >  fs/debugfs/file.c                     | 43 ++++++++++++++++++++++++++-
> >  include/linux/debugfs.h               | 11 +++++++
> >  3 files changed, 55 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/filesystems/debugfs.rst b/Documentation/filesystems/debugfs.rst
> > index dc35da8b8792..b2c76ac3a333 100644
> > --- a/Documentation/filesystems/debugfs.rst
> > +++ b/Documentation/filesystems/debugfs.rst
> > @@ -178,6 +178,8 @@ file::
> >
> >      void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
> >  			 int nregs, void __iomem *base, char *prefix);
> > +    void debugfs_print_regmap_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
> > +			 int nregs, struct regmap *regmap*, char *prefix);
>
> One too many "*" characters on that last line, right?

Good catch! I will remove it in V2.

> >
> >  The "base" argument may be 0, but you may want to build the reg32 array
> >  using __stringify, and a number of register names (macros) are actually
> > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > index b54f470e0d03..2fb792843b30 100644
> > --- a/fs/debugfs/file.c
> > +++ b/fs/debugfs/file.c
> > @@ -1137,14 +1137,55 @@ void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
> >  }
> >  EXPORT_SYMBOL_GPL(debugfs_print_regs32);
> >
> > +/**
> > + * debugfs_print_regmap_regs32 - use seq_print to describe a set of registers
> > + * @s: the seq_file structure being used to generate output
> > + * @regs: an array if struct debugfs_reg32 structures
> > + * @nregs: the length of the above array
> > + * @regmap: regmap to be used in reading the registers
> > + * @prefix: a string to be prefixed to every output line
> > + *
> > + * This function outputs a text block describing the current values of
> > + * some 32-bit hardware registers. It is meant to be used within debugfs
> > + * files based on seq_file that need to show registers, intermixed with other
> > + * information. The prefix argument may be used to specify a leading string,
> > + * because some peripherals have several blocks of identical registers,
> > + * for example configuration of dma channels
> > + */
> > +void debugfs_print_regmap_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
> > +			  int nregs, struct regmap *regmap, char *prefix)
> > +{
> > +	int i;
> > +	u32 val;
> > +
> > +	for (i = 0; i < nregs; i++, regs++) {
> > +		if (prefix)
> > +			seq_printf(s, "%s", prefix);
> > +		regmap_read(regmap, regs->offset, &val);
> > +		seq_printf(s, "%s = 0x%08x\n", regs->name, val);
> > +		if (seq_has_overflowed(s))
> > +			break;
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(debugfs_print_regmap_regs32);
> > +
> >  static int debugfs_regset32_show(struct seq_file *s, void *data)
> >  {
> >  	struct debugfs_regset32 *regset = s->private;
> >
> > +	void __iomem *base = regset->base;
> > +	struct regmap *regmap = regset->regmap;
>
> Why the extra blank line?  Did you run checkpatch?

Yeah, I do checkpatch & sparse(coccinelle crash somehow)
for all my patches(but forget to remove gerrit's Change-ID finally, sorry).

checkpatch didn't find this extra blank line.

I will remove it in v2, thanks!

> And it's generally not considered a good idea to dereference a pointer
> _before_ it is checked.  It will not crash, but static checkers will
> have a field day with it.

Ok, will check regset before access in v2.

> > +
> > +	if ((regmap && base) || (!regmap && !base))
> > +		return -EINVAL;
> > +
> >  	if (regset->dev)
> >  		pm_runtime_get_sync(regset->dev);
> >
> > -	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
> > +	if (base)
> > +		debugfs_print_regs32(s, regset->regs, regset->nregs, base, "");
> > +	else
> > +		debugfs_print_regmap_regs32(s, regset->regs, regset->nregs, regmap, "");
> >
> >  	if (regset->dev)
> >  		pm_runtime_put(regset->dev);
> > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> > index ea2d919fd9c7..87dfea6a25a0 100644
> > --- a/include/linux/debugfs.h
> > +++ b/include/linux/debugfs.h
> > @@ -17,6 +17,7 @@
> >
> >  #include <linux/types.h>
> >  #include <linux/compiler.h>
> > +#include <linux/regmap.h>
>
> No need to include this here, just provide a prototype for "struct
> regmap" and all will be fine.

Well noted, I will forward declare "struct regmap" in debugfs.h,
and move regmap.h to debugfs.c(regmap_read is used here).

Thanks for your review.

Regards,
Li

Re: [PATCH 00/15] Ambarella S6LM SoC bring-up

[Bagas Sanjaya]

On 1/23/23 15:39, Arnd Bergmann wrote:
> I seem to only have part of the series, please add both me and
> the linux-arm-kernel mailing list to each part of the initial
> submission.
> 
> It's possible that some patches were already Cc'd to
> linux-arm-kernel but did not make it through because the Cc list
> was too long (it has to fit within 1024 characters for many lists).
> I think you too the Cc list from get_maintainers.pl, but when
> sending new drivers this does not work well because it picks
> up everyone that recently touched the Makefile/Kconfig.

Hi Arnd,

It is possible (and common) that people who recently touched these
files, when given new drivers patches, aren't interested in reviewing
them for many reasons.

In that case, you may want to see Alison's trick posted on kernel
outreachy list [1]. In summary, pass `--no-gitfallback` (don't give
addresses of recent commit authors) and `--norolestats` (only name and
email are printed; MLs don't get open list:-generated names). Also,
another trick that I use is to condense the list by passing
`--separator , ` so that it can be easily copy-pasted to
git-send-email(1).

Thanks.

[1]: https://lore.kernel.org/outreachy/20211015171331.GA431883@alison-desk/

-- 
An old man doll... just what I always wanted! - Clara