Re: [PATCH] gpio: dwapb: Add support for 32 interrupts

[Andy Shevchenko]

On Wed, Mar 28, 2018 at 5:22 PM, Phil Edworthy
 wrote:
> The DesignWare GPIO IP can be configured for either 1 or 32 interrupts,

1 to 32, or just a choice between two?

> but the driver currently only supports 1 interrupt. See the DesignWare
> DW_apb_gpio Databook description of the 'GPIO_INTR_IO' parameter.

Will see after holiday and perhaps make more comments. Here is just a
brief review.

> +- interrupts : The interrupts to the parent controller raised when GPIOs
> +  generate the interrupts. If the controller provides one combined interrupt
> +  for all GPIOs, specify a single interrupt. If the controller provides one
> +  interrupt for each GPIO, provide a list of interrupts that correspond to 
> each
> +  of the GPIO pins. When specifying multiple interrupts, if any of the GPIOs 
> are
> +  not connected to an interrupt, use the interrupt-mask property.
> +- interrupt-mask : a 32-bit bit mask that specifies which interrupts in the 
> list
> +  of interrupts is valid, bit is 1 for a valid irq.

So, but why one will need that in practice? GPIO driver usually
provides a pin based IRQ chip which maps each pin to the corresponding
offset inside specific IRQ domain.

> +   struct device_node *np = to_of_node(fwnode);
> +   u32 irq_mask = 0x;

Why? Shouldn't it be dependent to the amount of actual pins / ports?
Intel Quark has only 8 AFAIR.

> +   int j;
> +
> +   /* Optional irq mask */
> +   fwnode_property_read_u32(fwnode, "interrupt-mask", 
> _mask);
> +
> +   /*
> +* The IP has configuration options to allow a single
> +* combined interrupt or one per gpio. If one per 
> gpio,
> +* some might not be used.
> +*/

> +   for (j = 0; j < pp->ngpio; j++) {
> +   if (irq_mask & BIT(j)) {

for_each_set_bit() is in kernel for ages!

> +   pp->irq[j] = irq_of_parse_and_map(np, 
> j);
> +   if (pp->irq[j])
> +   pp->has_irq = true;
> +   }
> +   }


So, on the first glance the patch looks either superfluous or taking
wrong approach. Please, elaborate more why it's done in this way and
what the case for all this in practice.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next] dt-bindings: net: renesas-ravb: Add support for r8a77470 SoC

[David Miller]

From: Biju Das 
Date: Thu, 29 Mar 2018 11:02:55 +0100

> Add a new compatible string for the RZ/G1C (R8A77470) SoC.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Applied.

Re: [PATCH/RFT v2 0/3] thermal: add support for r8a77995

[Simon Horman]

On Fri, Mar 30, 2018 at 12:13:00PM +0900, Yoshihiro Kaneko wrote:
> This series adds thermal support for r8a77995.
> R-Car D3 (r8a77995) have a thermal sensor module which is similar to Gen2.
> Therefore this series adds r8a77995 support to rcar_thermal driver not
> rcar_gen3_thermal driver.
> 
> This series is based on the next branch of Zhang Rui's linux tree.

I have very lightly tested this as follows after enabling
CONFIG_RCAR_THERMAL in the kernel .config.

# cat /sys/devices/virtual/thermal/thermal_zone0/temp
4

Re: [PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig

[kbuild test robot]

Hi Michel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on renesas/devel]
[also build test ERROR on v4.16-rc7 next-20180329]
[cannot apply to robh/for-next power-supply/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Michel-Pollet/arm-Base-support-for-Renesas-RZN1D-DB-Board/20180330-103029
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git devel
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/power/reset/rzn1-reboot.c: In function 'rzn1_reboot_probe':
>> drivers/power/reset/rzn1-reboot.c:70:2: warning: this 'if' clause does not 
>> guard... [-Wmisleading-indentation]
 if (!parent || !parent->of_node)
 ^~
   drivers/power/reset/rzn1-reboot.c:72:3: note: ...this statement, but the 
latter is misleadingly indented as if it were guarded by the 'if'
  return -ENODEV;
  ^~
   drivers/power/reset/rzn1-reboot.c:66:6: warning: unused variable 'err' 
[-Wunused-variable]
 int err;
 ^~~
   drivers/power/reset/rzn1-reboot.c: At top level:
>> drivers/power/reset/rzn1-reboot.c:74:2: warning: data definition has no type 
>> or storage class
 sysctrl = syscon_node_to_regmap(parent->of_node);
 ^~~
>> drivers/power/reset/rzn1-reboot.c:74:2: error: type defaults to 'int' in 
>> declaration of 'sysctrl' [-Werror=implicit-int]
>> drivers/power/reset/rzn1-reboot.c:74:2: error: conflicting types for 
>> 'sysctrl'
   drivers/power/reset/rzn1-reboot.c:37:23: note: previous declaration of 
'sysctrl' was here
static struct regmap *sysctrl;
  ^~~
>> drivers/power/reset/rzn1-reboot.c:74:34: error: 'parent' undeclared here 
>> (not in a function); did you mean 'pte_t'?
 sysctrl = syscon_node_to_regmap(parent->of_node);
 ^~
 pte_t
>> drivers/power/reset/rzn1-reboot.c:75:2: error: expected identifier or '(' 
>> before 'if'
 if (IS_ERR(sysctrl)) {
 ^~
   drivers/power/reset/rzn1-reboot.c:79:2: warning: data definition has no type 
or storage class
 err = register_restart_handler(_reboot_nb);
 ^~~
>> drivers/power/reset/rzn1-reboot.c:79:2: error: type defaults to 'int' in 
>> declaration of 'err' [-Werror=implicit-int]
>> drivers/power/reset/rzn1-reboot.c:79:8: error: initializer element is not 
>> constant
 err = register_restart_handler(_reboot_nb);
   ^~~~
   drivers/power/reset/rzn1-reboot.c:80:2: error: expected identifier or '(' 
before 'if'
 if (err) {
 ^~
>> drivers/power/reset/rzn1-reboot.c:85:2: error: expected identifier or '(' 
>> before 'return'
 return err;
 ^~
>> drivers/power/reset/rzn1-reboot.c:86:1: error: expected identifier or '(' 
>> before '}' token
}
^
   cc1: some warnings being treated as errors

vim +74 drivers/power/reset/rzn1-reboot.c

a673cd20 Michel Pollet 2018-03-29  63  
a673cd20 Michel Pollet 2018-03-29  64  static int rzn1_reboot_probe(struct 
platform_device *pdev)
a673cd20 Michel Pollet 2018-03-29  65  {
a673cd20 Michel Pollet 2018-03-29 @66   int err;
a673cd20 Michel Pollet 2018-03-29  67   struct device *parent;
a673cd20 Michel Pollet 2018-03-29  68  
a673cd20 Michel Pollet 2018-03-29  69   parent = pdev->dev.parent;
a673cd20 Michel Pollet 2018-03-29 @70   if (!parent || !parent->of_node)
a673cd20 Michel Pollet 2018-03-29  71   dev_err(>dev, "couldn't 
find sysctrl node\n");
a673cd20 Michel Pollet 2018-03-29 @72   return -ENODEV;
a673cd20 Michel Pollet 2018-03-29  73   }
a673cd20 Michel Pollet 2018-03-29 @74   sysctrl = 
syscon_node_to_regmap(parent->of_node);
a673cd20 Michel Pollet 2018-03-29 @75   if (IS_ERR(sysctrl)) {
a673cd20 Michel Pollet 2018-03-29  76   dev_err(>dev, "couldn't 
find find regmap\n");
a673cd20 Michel Pollet 2018-03-29  77   return PTR_ERR(sysctrl);
a673cd20 Michel Pollet 2018-03-29  78   }
a673cd20 Michel Pollet 2018-03-29 @79   err = 
register_restart_handler(_reboot_nb);
a673cd20 Michel Pollet 2018-03-29  80   if (err) {
a673cd20 Michel Pollet 2018-03-29  81   dev_err(>dev, "register 
restart handler failed(err=%d)\n",
a673cd20 Michel Pollet 2018-03-29  82   err);
a673cd20 Michel Pollet 2018-03-29  83   }
a673cd20 Michel Pollet 2018-03-29  84  
a673cd20 Michel Pollet 2018-03-29 @85   return err;
a673cd20 M

Re: [PATCH] ARM: debug-ll: Add support for r8a77470

[Geert Uytterhoeven]

On Thu, Mar 29, 2018 at 12:42 PM, Biju Das  wrote:
> Enable low-level debugging support for RZ/G1C (r8a77470). RZ/G1C uses
> SCIF1 for the debug console.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH net-next] dt-bindings: net: renesas-ravb: Add support for r8a77470 SoC

[Geert Uytterhoeven]

On Thu, Mar 29, 2018 at 12:02 PM, Biju Das  wrote:
> Add a new compatible string for the RZ/G1C (R8A77470) SoC.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFT v2 1/3] thermal: rcar_thermal: add r8a77995 support

[Geert Uytterhoeven]

Hi Kaneko-san,

On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko  wrote:
> Add support for R-Car D3 (r8a77995) thermal sensor.
>
> Signed-off-by: Yoshihiro Kaneko 

Thanks for your patch!

> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -58,10 +58,35 @@ struct rcar_thermal_common {
> spinlock_t lock;
>  };
>
> +enum rcar_thermal_type {
> +   RCAR_THERMAL,
> +   RCAR_GEN2_THERMAL,
> +   RCAR_GEN3_THERMAL,
> +};
> +
> +struct rcar_thermal_chip {
> +   int use_of_thermal;

This can be a single bit:

unsigned int use_of_thermal : 1;

> +   enum rcar_thermal_type type;

If you would add feature bits, you can get rid of rcar_thermal_type:

unsigned int has_filonoff : 1;
unsigned int has_enr : 1;
unsigned int needs_suspend_resume : 1;

The number of interrupts can be stored here, too.

> +};
> +
> +static const struct rcar_thermal_chip rcar_thermal = {
> +   .use_of_thermal = 0,
> +   .type = RCAR_THERMAL,

.has_filonoff = 1,
.has_enr = 0,
...
.nirqs = 1,

> @@ -190,7 +222,8 @@ static int rcar_thermal_update_temp(struct 
> rcar_thermal_priv *priv)
>  * enable IRQ
>  */
> if (rcar_has_irq_support(priv)) {
> -   rcar_thermal_write(priv, FILONOFF, 0);
> +   if (priv->chip->type != RCAR_GEN3_THERMAL)

if (priv->chip->has_filonoff)

> @@ -438,6 +471,9 @@ static int rcar_thermal_probe(struct platform_device 
> *pdev)
> struct rcar_thermal_priv *priv;
> struct device *dev = >dev;
> struct resource *res, *irq;
> +   struct rcar_thermal_chip *chip = ((struct rcar_thermal_chip *)

I don't think the cast is needed.

> @@ -457,19 +493,35 @@ static int rcar_thermal_probe(struct platform_device 
> *pdev)
> pm_runtime_enable(dev);
> pm_runtime_get_sync(dev);
>
> -   irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -   if (irq) {
> -   /*
> -* platform has IRQ support.
> -* Then, driver uses common registers
> -* rcar_has_irq_support() will be enabled
> -*/
> -   res = platform_get_resource(pdev, IORESOURCE_MEM, mres++);
> -   common->base = devm_ioremap_resource(dev, res);
> -   if (IS_ERR(common->base))
> -   return PTR_ERR(common->base);
> +   for (i = 0; i < nirq; i++) {

for (i = 0; i < priv->nirqs; i++) {

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFT v2 2/3] dt-bindings: thermal: rcar-thermal: add R8A77995 support

[Geert Uytterhoeven]

On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko  wrote:
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFT v2 3/3] arm64: dts: renesas: r8a77995: add thermal device support

[Geert Uytterhoeven]

On Fri, Mar 30, 2018 at 5:13 AM, Yoshihiro Kaneko  wrote:
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file

[Geert Uytterhoeven]

Hi Michel,

On Thu, Mar 29, 2018 at 9:47 AM, Michel Pollet
 wrote:
> This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC
> bare bone support.
>
> This currently only handles generic parts (gic, architected timer)
> and a UART.
> For simplicity sake, this also relies on the bootloader to set the
> pinctrl and clocks.
>
> Signed-off-by: Michel Pollet 

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi

As I said on IRC, I'm not so happy with this r9a06g0xx.dtsi file without
SoC-specific compatible values, as you have to override all of them in
r9a06g032.dtsi and r9a06g033.dtsi

Moreover, in this series you dropped r9a06g032.dtsi, so the devices no longer
have SoC-specific compatible values?

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 3/8] DT: arm: renesas,rzn1: add the RZ/N1 SoC and RZN1D-DB board

[Geert Uytterhoeven]

Hi Michel,

On Thu, Mar 29, 2018 at 9:46 AM, Michel Pollet
 wrote:
> This documents the RZ/N1 bindings for both the RZ/N1 and the RZN1D-DB
> board.
>
> Signed-off-by: Michel Pollet 

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -47,7 +47,10 @@ SoCs:
>  compatible = "renesas,r8a77980"
>- R-Car D3 (R8A77995)
>  compatible = "renesas,r8a77995"
> -
> +  - RZ/N1 Family (R9A06G032 & R9A06G033)
> +compatible = "renesas,rzn1"
> +  - RZ/N1D (R9A06G032)
> +compatible = "renesas,r9a06g032", "renesas,rzn1"

Is there any value in the family-specific "renesas,rzn1" compatible
value at the SoC
level? We don't have them for other Renesas SoC families.

We've been bitten before by values like "renesas,rspi-rz", not knowing
at that time
that Renesas was going to have multiple different RZ subfamilies.
Compare also e.g. R-Car W2H with other R-Car Gen2 SoCs...

Apart from that:
Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 2/8] DT: reset: renesas,rzn1-reboot: document RZ/N1 reboot driver

[Geert Uytterhoeven]

Hi Michel,

On Thu, Mar 29, 2018 at 9:46 AM, Michel Pollet
 wrote:
> The Renesas RZ/N1 Family (Part #R9A06G0xx) requires a driver
> as part of the sysctrl MFD to handle rebooting the CA7 cores.
> This documents the driver bindings.
>
> Signed-off-by: Michel Pollet 

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/renesas,rzn1-reboot.txt
> @@ -0,0 +1,20 @@
> +DT bindings for the Renesas RZ/N1 Reboot Driver
> +
> +== Reboot Driver Node ==
> +
> +The reboot driver is always a subnode of the system controller node, see
> +renesas,rzn1-sysctrl.txt for details.
> +
> +Bindings:
> ++ Required:
> +   compatible = "renesas,rzn1-reboot";

You should list the supported SoC-specific compatible values here.

Quoting what I said on IRC:
1) DT bindings. These should list all compatible values possible/used.
2) DTS: These should list all applicable compatible values,
from most-specific to least-specific (SoC-specific,
family-specific (if exists), generic (if exists))
3 Driver: These should list only the least specific that is
sufficient to get the job done. So usually we have the
family-specific only, except if an SoC needs to be handled
specially, or for historical reasons (DTB backeards
compatibility)

> +
> +Example:
> +   sysctrl: sysctrl@4000c000 {
> +   compatible = "renesas,rzn1-sysctrl", "syscon", "simple-mfd";

Missing SoC-specific compatible value.

> +   reg = <0x4000c000 0x1000>;
> +
> +   reboot {
> +   compatible = "renesas,rzn1-reboot";

Missing SoC-specific compatible value.

> +   };
> +   };

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 1/8] DT: mfd: renesas,rzn1-sysctrl: document RZ/N1 sysctrl node

[Geert Uytterhoeven]

Hi Michel,

On Thu, Mar 29, 2018 at 9:46 AM, Michel Pollet
 wrote:
> The Renesas RZ/N1 Family (Part #R9A06G0xx) has a multi-function
> system controller. This documents the node used to encapsulate
> it's sub drivers.
>
> Signed-off-by: Michel Pollet 

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt
> @@ -0,0 +1,19 @@
> +DT bindings for the Renesas RZ/N1 System Controller
> +
> +== System Controller Node ==
> +
> +The system controller node currently only hosts a single sub-node to handle
> +the rebooting of the CPU. Eventually it will host the clock driver, SMP
> +start handler, watchdog etc.
> +
> +See renesas,rzn1-reboot.txt for further details.
> +
> +Bindings:
> ++ Required:
> +   compatible = "renesas,rzn1-sysctrl", "syscon", "simple-mfd";

You should list the supported SoC-specific compatible values here.

> +
> +Example:
> +   sysctrl: sysctrl@4000c000 {
> +   compatible = "renesas,rzn1-sysctrl", "syscon", "simple-mfd";

Missing SoC-specific compatible value.

> +   reg = <0x4000c000 0x1000>;
> +   };

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] dt-bindings: irqchip: renesas-irqc: Document r8a77470 support

[Geert Uytterhoeven]

On Thu, Mar 29, 2018 at 12:17 PM, Biju Das  wrote:
> Renesas RZ/G SoC have the R-Car gen2 compatible IRQC interrupt
> controllers. Document RZ/G1C (also known as R8A77470) SoC bindings.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] dt-bindings: rcar-dmac: Document r8a77470 support

[Geert Uytterhoeven]

On Thu, Mar 29, 2018 at 12:11 PM, Biju Das  wrote:
> Renesas  RZ/G SoC also have the R-Car gen2/3 compatible DMA controllers.
> Document RZ/G1C (also known as R8A77470) SoC bindings.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 0/5] clk: renesas: r-car gen2: Fix LB clock divider

[Simon Horman]

On Thu, Mar 29, 2018 at 07:33:05PM +0200, Geert Uytterhoeven wrote:
>   Hi all,
> 
> The CLK_TYPE_GEN2_LB clock type is meant for SoCs like R-Car H2, where
> the LB clock divider depends on the value of the MD18 pin.
> 
> However, on most RZ/G1 and R-Car Gen2 SoCs, the LB clock divider is
> fixed to 24.  Hence this series corrects the LB clock on affected SoCs
> by modelling it as a fixed factor clock instead.
> 
> This doesn't have much impact, as no kernel code relies on the rate of
> the LB clock.
> 
> To be queued in clk-renesas-for-v4.18.

Reviewed-by: Simon Horman 

Re: [PATCH v3 8/8] DT: arm: Add the RZN1D-DB Board to Renesas Makefile target

[Simon Horman]

On Thu, Mar 29, 2018 at 08:47:04AM +0100, Michel Pollet wrote:
> This adds the newly added board to the Renesas built target
> 
> Signed-off-by: Michel Pollet 
> Reviewed-by: Geert Uytterhoeven 
> ---
>  arch/arm/boot/dts/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 8164c12..1849228 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -806,6 +806,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \
>   r8a7793-gose.dtb \
>   r8a7794-alt.dtb \
>   r8a7794-silk.dtb \
> + r9a06g032-rzn1d400-db.dtb \
>   sh73a0-kzm9g.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += \
>   rv1108-evb.dtb \

I think you can squash this patch into the one that adds the corresponding
dts file.

Re: [PATCH v3 7/8] DT: arm: Add Renesas RZN1D-DB Board base file

[Simon Horman]

On Thu, Mar 29, 2018 at 08:47:03AM +0100, Michel Pollet wrote:
> This adds a base device tree file for the RZN1-DB board, with only the
> basic support allowing the system to boot to a prompt. Only one UART is
> used, with only a single CPU running.
> 
> Signed-off-by: Michel Pollet 
> ---
>  arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts | 26 ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts
> 
> diff --git a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts 
> b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts
> new file mode 100644
> index 000..a462b1a
> --- /dev/null
> +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the RZN1D-DB Board
> + *
> + * Copyright (C) 2018 Renesas Electronics Europe Limited
> + *
> + */
> +
> +/dts-v1/;
> +
> +#include "r9a06g0xx.dtsi"
> +
> +/ {
> + model = "RZN1D-DB Board";
> + compatible = "renesas,rzn1d400-db", "renesas,r9a06g032", "renesas,rzn1";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> + aliases {
> + serial0 = 
> + };
> +};
> + {
> + status = "okay";
> +};

Please add a blank line between nodes.
Please use "ARM: dts: " as the patch prefix.

Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file

[Simon Horman]

On Thu, Mar 29, 2018 at 01:04:50PM +0200, jacopo mondi wrote:
> Hi Michel
> 
> The subject of all your patches for arch/arm should start with:
> 
> ARM: dts:
> 
> A git log on that directory clearly shows that's the preferred one.
> 
> I would also say that you are missing a symbol definition in
> arch/arm/mach-shmobile/Kconfig
> (even if you got rid of any board file)
> 
> I would expect a symbol to select in menuconfig, with your
> dependencies listed there (ie, the serial interface driver)
> 
> Something like this (I left the 'xx' out from the part name on purpose)
> 
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 280e731..9a519330 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -114,4 +114,8 @@ config ARCH_SH73A0
> bool "SH-Mobile AG5 (R8A73A00)"
> select ARCH_RMOBILE
> select RENESAS_INTC_IRQPIN
> +
> +config ARCH_R9A06G0
> +   bool "RZ/N1 (R9A06G0)"
> +   select SERIAL_8250_DW
>  endif
> 
> But please wait for others (preferibly Geert or Simon) to confim this.

I think that is covered by
"[PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig"

> On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote:
> > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC
> > bare bone support.
> >
> > This currently only handles generic parts (gic, architected timer)
> > and a UART.
> > For simplicity sake, this also relies on the bootloader to set the
> > pinctrl and clocks.
> >
> > Signed-off-by: Michel Pollet 
> > ---
> >  arch/arm/boot/dts/r9a06g0xx.dtsi | 96 
> > 
> >  1 file changed, 96 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi
> >
> > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi 
> > b/arch/arm/boot/dts/r9a06g0xx.dtsi
> > new file mode 100644
> > index 000..c63
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi
> > @@ -0,0 +1,96 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Europe Limited
> > + *
> > + */
> > +
> > +#include 
> > +
> > +/ {
> > +   compatible = "renesas,rzn1";
> > +   #address-cells = <1>;
> > +   #size-cells = <1>;
> > +
> > +   cpus {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   cpu@0 {
> > +   device_type = "cpu";
> > +   compatible = "arm,cortex-a7";
> > +   reg = <0>;
> > +   };
> > +   cpu@1 {
> > +   device_type = "cpu";
> > +   compatible = "arm,cortex-a7";
> > +   reg = <1>;
> > +   };
> > +   };
> 
> I see you don't like empy lines, that's fine, it is not a strict
> requiremen afaik, but I find a few empty lines here and there more
> redable, expecially if the file is going to grow, as it will be.

Please place one empty line between each node.

> > +   clocks {
> > +   /*
> > +* this is fixed clock for now,
> > +* until the clock driver is merged
> > +*/
> > +   clkuarts: clkuarts {
> 
> You can remove the node lable if it's the same as the node name afaik
> 
> > +   #clock-cells = <0>;
> > +   compatible = "fixed-clock";
> > +   clock-frequency = <47619047>;
> > +   };
> > +   };
> 
> Grouping clock nodes under a "clocks" one is now deprecated.
> 
> Please see, ie.
> "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode"

Also, please sort sub-nodes of the root node alphabetically.

> Thanks
>j
> 
> > +   arch-timer {
> > +   compatible = "arm,cortex-a7-timer",
> > +"arm,armv7-timer";
> > +   interrupt-parent = <>;
> > +   arm,cpu-registers-not-fw-configured;
> > +   interrupts =
> > +> +   IRQ_TYPE_LEVEL_LOW)>,
> > +> +   IRQ_TYPE_LEVEL_LOW)>,
> > +> +   IRQ_TYPE_LEVEL_LOW)>,
> > +> +   IRQ_TYPE_LEVEL_LOW)>;

I think its nicer not to line-wrap the individual interrupts.
I.e. please make the above like this:

interrupts =
,
,
,
;

> > +   };
> > +   soc {
> > +   compatible = "simple-bus";
> > +   #address-cells = <1>;
> > +   #size-cells = <1>;
> > +   interrupt-parent = <>;
> > +   ranges;
> > +
> > +   gic: gic@44101000 {

Please sort subnodes of the soc node using:
- Primary key of bus address
- Secondary key of IP block type (does not apply here)


> > +   compatible = "arm,cortex-a7-gic", 

Re: [PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig

[Simon Horman]

On Thu, Mar 29, 2018 at 08:47:01AM +0100, Michel Pollet wrote:
> Add the RZ/N1 Family (Part #R9A06G0xx) ARCH config to the rest of
> the Renesas SoC collection.
> 
> Signed-off-by: Michel Pollet 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied.

Re: [PATCH] dt-bindings: irqchip: renesas-irqc: Document r8a77470 support

[Simon Horman]

On Thu, Mar 29, 2018 at 11:17:08AM +0100, Biju Das wrote:
> Renesas RZ/G SoC have the R-Car gen2 compatible IRQC interrupt
> controllers. Document RZ/G1C (also known as R8A77470) SoC bindings.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Reviewed-by: Simon Horman 

Re: [PATCH] dt-bindings: rcar-dmac: Document r8a77470 support

[Simon Horman]

On Thu, Mar 29, 2018 at 11:11:06AM +0100, Biju Das wrote:
> Renesas  RZ/G SoC also have the R-Car gen2/3 compatible DMA controllers.
> Document RZ/G1C (also known as R8A77470) SoC bindings.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 

Reviewed-by: Simon Horman 

Re: [PATCH v2 8/8] ARM: multi_v7_defconfig: Enable r8a77470 SoC

[Simon Horman]

On Thu, Mar 29, 2018 at 09:15:03AM +0200, Geert Uytterhoeven wrote:
> On Wed, Mar 28, 2018 at 9:26 PM, Biju Das  wrote:
> > Enable recently added r8a77470 (RZ/G1C) SoC.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Fabrizio Castro 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied.

Re: [PATCH 09/12] ARM: shmobile: Document iW-RainboW-G23S single board computer

[Simon Horman]

On Thu, Mar 29, 2018 at 10:00:15AM +, Fabrizio Castro wrote:
> Hello Simon,
> 
> thank you for reworking the subject.
> 
> > Subject: Re: [PATCH 09/12] ARM: shmobile: Document iW-RainboW-G23S single 
> > board computer
> >
> > On Wed, Mar 28, 2018 at 09:36:10AM +0200, Geert Uytterhoeven wrote:
> > > On Tue, Mar 27, 2018 at 4:37 PM, Biju Das  wrote:
> > > >
> > > > Document the iW-RainboW-G23S single board computer device tree bindings,
> > > > listing it as a supported board.
> > > >
> > > > Signed-off-by: Biju Das 
> > > > Reviewed-by: Fabrizio Castro 
> > >
> > > Reviewed-by: Geert Uytterhoeven 
> >
> > Thanks, applied with the subject updated to:
> >
> > dt-bindings: arm:: Document iW-RainboW-G23S single board computer
> 
> There is an extra : in the new subject, is it too late for fixing this?

Thanks, I will fix that.

Re: [PATCH v2 6/8] ARM: dts: r8a77470: Initial SoC device tree

[Simon Horman]

On Wed, Mar 28, 2018 at 08:26:14PM +0100, Biju Das wrote:
> The initial R8A77470 SoC device tree including CPU0, GIC, timer, SYSC, RST,
> CPG, and the required clock descriptions.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> Reviewed-by: Geert Uytterhoeven 
> ---
> V1->V2:
> * Incorporated geert's review comment
> * Moved prr node inside soc node.
> 
>  arch/arm/boot/dts/r8a77470.dtsi | 155 
> 
>  1 file changed, 155 insertions(+)
>  create mode 100644 arch/arm/boot/dts/r8a77470.dtsi
> 
> diff --git a/arch/arm/boot/dts/r8a77470.dtsi b/arch/arm/boot/dts/r8a77470.dtsi
> new file mode 100644
> index 000..f096419
> --- /dev/null
> +++ b/arch/arm/boot/dts/r8a77470.dtsi
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the r8a77470 SoC
> + *
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 

As requested by Geert in his review of v1, please use numerical values for
the initial submission as the dt-bindings headers and the DTS files go
upstream through different maintainer paths.

In other words, please do not include r8a77470-cpg-mssr.h or
r8a77470-sysc.h in the initial submission of this file as those header
files will not be present and there will be a build failure.
Rather, please use numeric values for now and, as a follow-up in the v4.19
development cycle, provide a patch to use the headers.

> +/ {
> + compatible = "renesas,r8a77470";
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0>;
> + clock-frequency = <10>;
> + clocks = < CPG_CORE R8A77470_CLK_Z2>;
> + power-domains = < R8A77470_PD_CA7_CPU0>;
> + next-level-cache = <_CA7>;
> + };
> +
> +
> + L2_CA7: cache-controller-0 {
> + compatible = "cache";
> + cache-unified;
> + cache-level = <2>;
> + power-domains = < R8A77470_PD_CA7_SCU>;
> + };
> + };
> +
> + /* External root clock */
> + extal_clk: extal {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + /* This value must be overridden by the board. */
> + clock-frequency = <0>;
> + };
> +
> + /* External SCIF clock */
> + scif_clk: scif {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + /* This value must be overridden by the board. */
> + clock-frequency = <0>;
> + };
> +
> + soc {
> + compatible = "simple-bus";
> + interrupt-parent = <>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + cpg: clock-controller@e615 {
> + compatible = "renesas,r8a77470-cpg-mssr";
> + reg = <0 0xe615 0 0x1000>;
> + clocks = <_clk>, <_extal_clk>;
> + clock-names = "extal", "usb_extal";
> + #clock-cells = <2>;
> + #power-domain-cells = <0>;
> + #reset-cells = <1>;
> + };
> +
> + rst: reset-controller@e616 {
> + compatible = "renesas,r8a77470-rst";
> + reg = <0 0xe616 0 0x100>;
> + };
> +
> + sysc: system-controller@e618 {
> + compatible = "renesas,r8a77470-sysc";
> + reg = <0 0xe618 0 0x200>;
> + #power-domain-cells = <1>;
> + };
> +
> + icram0: sram@e63a {
> + compatible = "mmio-sram";
> + reg = <0 0xe63a 0 0x12000>;
> + };
> +
> + icram1: sram@e63c {
> + compatible = "mmio-sram";
> + reg = <0 0xe63c 0 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0xe63c 0x1000>;
> +
> + smp-sram@0 {
> + compatible = "renesas,smp-sram";
> + reg = <0 0x100>;
> + };
> + };
> +
> + icram2: sram@e630 {
> + compatible = "mmio-sram";
> + reg = <0 0xe630 0 0x2>;
> + };
> +
> + scif1: serial@e6e68000 {
> + compatible = "renesas,scif-r8a77470",
> 

Re: [PATCH v2 5/8] ARM: shmobile: r8a77470: basic SoC support

[Simon Horman]

On Thu, Mar 29, 2018 at 09:15:05AM +0200, Geert Uytterhoeven wrote:
> On Wed, Mar 28, 2018 at 9:26 PM, Biju Das  wrote:
> > Add minimal support for the RZ/G1C (R8A77470) SoC.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Fabrizio Castro 
> > ---
> > V1->V2:
> > * No change
> 
> Hence my
> Reviewed-by: Geert Uytterhoeven 
> for v1 is still valid.

Thanks, applied.

Re: [PATCH v2 1/8] soc: renesas: rcar-sysc: Add r8a77470 support

[Simon Horman]

On Wed, Mar 28, 2018 at 08:26:09PM +0100, Biju Das wrote:
> Add support for RZ/G1C (R8A77470) SoC power areas to the R-Car SYSC
> driver.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied.