Re: [PATCH/RFC] ARM: shmobile: Always enable ARCH_TIMER on SoCs with A7 and/or A15
On Fri, Jul 13, 2018 at 1:08 AM, Geert Uytterhoeven wrote: > R-Mobile APE6, R-Car Gen2, and RZ/G1 SoCs have Cortex-A7 and/or > Cortex-A15 CPU cores, all of which have ARM architectured timers. > > Force use of the ARM architectured timer on these SoCs. > This allows to: > - Remove the calls to shmobile_init_delay() from the corresponding > machine vectors, > - Remove a check in timer setup specific to R-Car Gen2, > - Remove a check in shmobile_init_delay(). > > Signed-off-by: Geert Uytterhoeven > --- Hi Geert, Thanks for cleaning up stuff. > We still need shmobile_init_delay to setup loops-per-jiffies for the > other SoCs. But perhaps we can use the Cortex-A9 global timer on those? Perhaps unrelated, but on older kernels with Cortex-A9 the TWD timer was only available when using SMP. So historically on our single-core systems (with CA8 or CA9 or when using CA9 multi core with maxcpus=1) we had no timer available unless we also used a CMT/TMU or similar. Exactly what the level of software support is available for the ARM timers at this point I'm not so sure about. Cheers, / magnus
Re: [PATCH v2 1/3] i2c: recovery: if possible send STOP with recovery pulses
On Tue, Jul 10, 2018 at 11:42:15PM +0200, Wolfram Sang wrote: > I2C clients may misunderstand recovery pulses if they can't read SDA to > bail out early. In the worst case, as a write operation. To avoid that > and if we can write SDA, try to send STOP to avoid the > misinterpretation. > > Signed-off-by: Wolfram Sang Applied to for-current, thanks! signature.asc Description: PGP signature
[PATCH] i2c: recovery: make pin init look like STOP
When we we initialize the pins, make sure it looks like STOP by dividing the delay into halves. It shouldn't matter because SDA is expected to be held low by a device, but for super-safety, let's do it. Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-base.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 51cbb0c158f2..e57231ccb32a 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -191,9 +191,10 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap) bri->prepare_recovery(adap); bri->set_scl(adap, scl); + ndelay(RECOVERY_NDELAY / 2); if (bri->set_sda) - bri->set_sda(adap, 1); - ndelay(RECOVERY_NDELAY); + bri->set_sda(adap, scl); + ndelay(RECOVERY_NDELAY / 2); /* * By this time SCL is high, as we need to give 9 falling-rising edges -- 2.11.0
Re: [PATCH 1/2] i2c: recovery: add get_bus_free callback
> > More importantly, isn't a ->get_bus_free implementation a sufficient > > requirement > > for recovery? I.e. even if both ->get_sda and ->set_sda are missing? > > In my case, it isn't. It needs set_sda (== STOP) to achieve a useful > result. Hmm, I think this needs better documentation... On a second thought, it may be possible to simply merge get_sda and get_bus_free in the future. For now, I'd like to keep them seperate until we have implemented bus recovery for some more hardware. signature.asc Description: PGP signature
RE: [PATCH 1/2] ARM: shmobile: Add basic RZ/A2 SoC support
Hi Geert, On Thursday, July 12, 2018, Geert Uytterhoeven wrote: > Yeah, that's why I asked: setup-r7s72100.c is the smallest setup file. > > It uses shmobile_init_delay() to preset loops-per-jiffy, to avoid > calibrating the > delay loop, and shmobile_init_late() to make s2ram do more than s2idle. > As RZ/A doesn't have SMP, and thus there's no use for disabling secondary > CPU cores, the impact of the latter is small (read: I don't know what's > the real > impact of calling cpu_idle_poll_ctrl()). It's been quite a while, but I remember when porting RZ/A1 code that without those two functions, things didn't work right. Of course that was many releases ago. However, one thing that I know I need is + .l2c_aux_val= 0, + .l2c_aux_mask = ~0, Without that, I don't get my L2C driver loaded. Hence: a96bb197693e ("ARM: 8660/1: shmobile: r7s72100: Enable L2 cache") So at least for now, I would say I still need the setup file. Chris
Re: [PATCH 1/2] ARM: shmobile: Add basic RZ/A2 SoC support
Hi Chris, On Thu, Jul 12, 2018 at 5:40 PM Chris Brandt wrote: > On Thursday, July 12, 2018, Geert Uytterhoeven wrote: > > I'm wondering if you could do without any board code, i.e. without > > setup-r7s9210.c? > > I think I see them being removed for R-Car. > ButI'm not sure how that actually works. > > I'll have a look. > > As you can see, there's really nothing in the RZ/A1 setup file either. Yeah, that's why I asked: setup-r7s72100.c is the smallest setup file. It uses shmobile_init_delay() to preset loops-per-jiffy, to avoid calibrating the delay loop, and shmobile_init_late() to make s2ram do more than s2idle. As RZ/A doesn't have SMP, and thus there's no use for disabling secondary CPU cores, the impact of the latter is small (read: I don't know what's the real impact of calling cpu_idle_poll_ctrl()). 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] gpiolib: use better errno if get_direction is not available
> For the short term, I will remove the sanity check from the I2C core and > hope the user properly set up the GPIO. For the record, this patch is here: http://patchwork.ozlabs.org/patch/943115/ Forgot to cc linux-gpio. signature.asc Description: PGP signature
[PATCH] i2c: recovery: remove bogus check if SDA GPIO is set to output
This check did not work as intended. I2C is open drain, so this function will likely always have presented the GPIO as input because gpiod_get_direction doesn't know about open drain states. Remove this check for now. We can add it again once we know how to get more precise information about the GPIO. Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-base.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 57538d72f2e5..4debf921ab5c 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -269,9 +269,7 @@ static void i2c_init_recovery(struct i2c_adapter *adap) bri->set_scl = set_scl_gpio_value; if (bri->sda_gpiod) { bri->get_sda = get_sda_gpio_value; - /* FIXME: add proper flag instead of '0' once available */ - if (gpiod_get_direction(bri->sda_gpiod) == 0) - bri->set_sda = set_sda_gpio_value; + bri->set_sda = set_sda_gpio_value; } return; } -- 2.11.0
Re: [PATCH] gpio: rcar: Implement .get_direction() callback
On Thu, Jul 12, 2018 at 11:15:01AM +0200, Geert Uytterhoeven wrote: > Allow gpiolib to read back the current I/O direction configuration by > implementing the .get_direction() callback. > > Signed-off-by: Geert Uytterhoeven So, despite it didn't help solving my original case because of an unrelated issue, I verified that this code works: Reviewed-by: Wolfram Sang Tested-by: Wolfram Sang I'd still think it is better to have this code than to not have it. Thanks Geert! Regards, Wolfram signature.asc Description: PGP signature
[PATCH/RFC] ARM: shmobile: Always enable ARCH_TIMER on SoCs with A7 and/or A15
R-Mobile APE6, R-Car Gen2, and RZ/G1 SoCs have Cortex-A7 and/or Cortex-A15 CPU cores, all of which have ARM architectured timers. Force use of the ARM architectured timer on these SoCs. This allows to: - Remove the calls to shmobile_init_delay() from the corresponding machine vectors, - Remove a check in timer setup specific to R-Car Gen2, - Remove a check in shmobile_init_delay(). Signed-off-by: Geert Uytterhoeven --- We still need shmobile_init_delay to setup loops-per-jiffies for the other SoCs. But perhaps we can use the Cortex-A9 global timer on those? arch/arm/mach-shmobile/Kconfig | 2 ++ arch/arm/mach-shmobile/setup-r8a73a4.c | 1 - arch/arm/mach-shmobile/setup-rcar-gen2.c | 4 arch/arm/mach-shmobile/timer.c | 8 4 files changed, 2 insertions(+), 13 deletions(-) diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig index 0b67254eabb2c4e8..aeb2eed085988bb8 100644 --- a/arch/arm/mach-shmobile/Kconfig +++ b/arch/arm/mach-shmobile/Kconfig @@ -15,6 +15,7 @@ config ARCH_RCAR_GEN1 config ARCH_RCAR_GEN2 bool + select HAVE_ARM_ARCH_TIMER select PM select PM_GENERIC_DOMAINS select RENESAS_IRQC @@ -58,6 +59,7 @@ config ARCH_R8A73A4 bool "R-Mobile APE6 (R8A73A40)" select ARCH_RMOBILE select ARM_ERRATA_798181 if SMP + select HAVE_ARM_ARCH_TIMER select RENESAS_IRQC config ARCH_R8A7740 diff --git a/arch/arm/mach-shmobile/setup-r8a73a4.c b/arch/arm/mach-shmobile/setup-r8a73a4.c index ef391fa368e9ad00..23a29a0ea9c96c35 100644 --- a/arch/arm/mach-shmobile/setup-r8a73a4.c +++ b/arch/arm/mach-shmobile/setup-r8a73a4.c @@ -18,7 +18,6 @@ static const char *const r8a73a4_boards_compat_dt[] __initconst = { }; DT_MACHINE_START(R8A73A4_DT, "Generic R8A73A4 (Flattened Device Tree)") - .init_early = shmobile_init_delay, .init_late = shmobile_init_late, .dt_compat = r8a73a4_boards_compat_dt, MACHINE_END diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c index 117531367f1779c6..013acc97795cbfc4 100644 --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c @@ -59,7 +59,6 @@ static unsigned int __init get_extal_freq(void) void __init rcar_gen2_timer_init(void) { -#ifdef CONFIG_ARM_ARCH_TIMER void __iomem *base; u32 freq; @@ -101,7 +100,6 @@ void __init rcar_gen2_timer_init(void) } iounmap(base); -#endif /* CONFIG_ARM_ARCH_TIMER */ of_clk_init(NULL); timer_probe(); @@ -187,7 +185,6 @@ static const char * const rcar_gen2_boards_compat_dt[] __initconst = { }; DT_MACHINE_START(RCAR_GEN2_DT, "Generic R-Car Gen2 (Flattened Device Tree)") - .init_early = shmobile_init_delay, .init_late = shmobile_init_late, .init_time = rcar_gen2_timer_init, .reserve= rcar_gen2_reserve, @@ -202,7 +199,6 @@ static const char * const rz_g1_boards_compat_dt[] __initconst = { }; DT_MACHINE_START(RZ_G1_DT, "Generic RZ/G1 (Flattened Device Tree)") - .init_early = shmobile_init_delay, .init_late = shmobile_init_late, .init_time = rcar_gen2_timer_init, .reserve= rcar_gen2_reserve, diff --git a/arch/arm/mach-shmobile/timer.c b/arch/arm/mach-shmobile/timer.c index 6196a63803853048..828e8aea037e7d5e 100644 --- a/arch/arm/mach-shmobile/timer.c +++ b/arch/arm/mach-shmobile/timer.c @@ -32,14 +32,6 @@ void __init shmobile_init_delay(void) for_each_child_of_node(cpus, np) { u32 freq; - if (IS_ENABLED(CONFIG_ARM_ARCH_TIMER) && - (of_device_is_compatible(np, "arm,cortex-a7") || -of_device_is_compatible(np, "arm,cortex-a15"))) { - of_node_put(np); - of_node_put(cpus); - return; - } - if (!of_property_read_u32(np, "clock-frequency", )) max_freq = max(max_freq, freq); } -- 2.17.1
Re: [PATCH] gpiolib: use better errno if get_direction is not available
On Wed, Jul 11, 2018 at 06:33:19PM +0200, Wolfram Sang wrote: > EINVAL is very generic, use ENOTSUPP in case the gpiochip does not > provide this function. While removing the assignment from the 'status' > variable, use better indentation in the declaration block. > > Signed-off-by: Wolfram Sang So, Geert implemented get_direction for gpio-rcar, but this doesn't help my case sadly. Because I2C is open drain, get_direction returns 'input' :( As mentioned in a previous discussion, returning '0' and '1' from get_direction() is confusing. But as it looks now, it also is not enough. Or we'd need another function from which I could determine the OUT_OPEN_DRAIN state. For the short term, I will remove the sanity check from the I2C core and hope the user properly set up the GPIO. It would be nice to check for it somewhen in the future, though. That all being said, I think this patch is still useful as is. Thanks, Wolfram > --- > > I got puzzled by the EINVAL until I found out that gpio-rcar simply does not > implement it. > > @Geert: any reason gpio-rcar is missing it? I would need it for the > i2c-gpio-fault-injector. > > drivers/gpio/gpiolib.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index e11a3bb03820..18719f64e80b 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -210,15 +210,15 @@ static int gpiochip_find_base(int ngpio) > */ > int gpiod_get_direction(struct gpio_desc *desc) > { > - struct gpio_chip*chip; > - unsignedoffset; > - int status = -EINVAL; > + struct gpio_chip *chip; > + unsigned offset; > + int status; > > chip = gpiod_to_chip(desc); > offset = gpio_chip_hwgpio(desc); > > if (!chip->get_direction) > - return status; > + return -ENOTSUPP; > > status = chip->get_direction(chip, offset); > if (status > 0) { > -- > 2.11.0 > signature.asc Description: PGP signature
RE: [PATCH 2/2] dt-bindings: arm: Document RZ/A2 SoC DT bindings
Hi Geert, On Thursday, July 12, 2018, Geert Uytterhoeven wrote: > As the IP cores are the same in all variants, using > "renesas,r7s9210-" should be fine for matching drivers to IP > cores. Same for CONFIG_ARCH_R7S9210. > > However, as the actual dies differ between H, M, and L versions, there may > be integration issues to be worked around. So I think it would be wise to > use one more digit in the compatible value at the main SoC level, i.e. > "renesas,r7s92104". > Unless there's a hardware register to detect the version at runtime. > But it seems RZ/A2 doesn't have a Product Register > (PRR), which most other SH/R-Mobile and R-Car SoCs do have? There is supposed to be one. I specifically asked for it (and remember reviewing it). But, now I'm not seeing it in the latest version of the manual. Let me find out where it is. Chris
RE: [PATCH 1/2] ARM: shmobile: Add basic RZ/A2 SoC support
Hi Geert, On Thursday, July 12, 2018, Geert Uytterhoeven wrote: > I'm wondering if you could do without any board code, i.e. without > setup-r7s9210.c? I think I see them being removed for R-Car. ButI'm not sure how that actually works. I'll have a look. As you can see, there's really nothing in the RZ/A1 setup file either. Chris
Re: [PATCH 00/11] clk: renesas: rcar-gen3: OSC and RCLK improvements
On Wed, Jul 11, 2018 at 04:20:49PM +0200, Geert Uytterhoeven wrote: > Hi all, > > This patch series contains various improvements for OSC and RCLK > handling on R-Car Gen3 SoCs. > > This has been tested on R-Car H3 ES1.0/ES2.0, R-Car M3-W ES1.0, R-Car > D3, and R-Car E3. > The R-Car V3H (r8a77980) patches were compile-tested only due to lack of > hardware. > > Thanks for your comments! I made a few minor comments but nothing else stood out. Reviewed-by: Simon Horman > > Geert Uytterhoeven (11): > clk: renesas: rcar-gen3: Add support for OSC EXTAL predivider > clk: renesas: r8a7795: Add OSC EXTAL predivider configuration > clk: renesas: r8a7796: Add OSC EXTAL predivider configuration > clk: renesas: r8a77965: Add OSC EXTAL predivider configuration > clk: renesas: r8a77980: Add OSC predivider configuration and clock > clk: renesas: cpg-mssr: Add support for fixed rate clocks > clk: renesas: rcar-gen3: Add support for RCKSEL clock selection > clk: renesas: r8a77990: Correct RCLK handling > clk: renesas: r8a77995: Correct RCLK handling > clk: renesas: rcar-gen3: Add support for mode pin clock selection > clk: renesas: r8a77980: Add RCLK for watchdog timer > > drivers/clk/renesas/r8a7795-cpg-mssr.c | 66 - > drivers/clk/renesas/r8a7796-cpg-mssr.c | 66 - > drivers/clk/renesas/r8a77965-cpg-mssr.c | 66 - > drivers/clk/renesas/r8a77980-cpg-mssr.c | 28 ++- > drivers/clk/renesas/r8a77990-cpg-mssr.c | 9 +++- > drivers/clk/renesas/r8a77995-cpg-mssr.c | 9 +++- > drivers/clk/renesas/rcar-gen3-cpg.c | 40 +++ > drivers/clk/renesas/rcar-gen3-cpg.h | 25 -- > drivers/clk/renesas/renesas-cpg-mssr.c | 5 ++ > drivers/clk/renesas/renesas-cpg-mssr.h | 3 ++ > 10 files changed, 189 insertions(+), 128 deletions(-) > > -- > 2.17.1 > > 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 05/11] clk: renesas: r8a77980: Add OSC predivider configuration and clock
Hi Simon, On Thu, Jul 12, 2018 at 3:56 PM Simon Horman wrote: > On Wed, Jul 11, 2018 at 04:20:54PM +0200, Geert Uytterhoeven wrote: > > R-Car Gen3 Hardware Manual Rev.0.54 documents the relation between the > > MD13 and MD14 mode pins, and the OSC EXTAL predivider, as used by the > > OSC clock. Hence augment the configuration structure with all > > documented predivider values. > > > > Add the OSC clock using the configured predivider. > > > > Signed-off-by: Geert Uytterhoeven > > --- a/drivers/clk/renesas/r8a77980-cpg-mssr.c > > +++ b/drivers/clk/renesas/r8a77980-cpg-mssr.c > > @@ -171,23 +173,23 @@ static const unsigned int r8a77980_crit_mod_clks[] > > __initconst = { > > */ > > > > /* > > - * MD EXTAL PLL2PLL1PLL3 > > + * MD EXTAL PLL2PLL1PLL3OSC > > * 14 13 (MHz) > > - * -- > > - * 0 0 16.66 x 1 x240x192x192 > > - * 0 1 20x 1 x200x160x160 > > - * 1 0 27x 1 x148x118x118 > > - * 1 1 33.33 / 2 x240x192x192 > > + * > > + * 0 0 16.66 x 1 x240x192x192/16 > > + * 0 1 20x 1 x200x160x160/19 > > + * 1 0 27x 1 x148x118x118/24 > > Not related to this patch, but in the Users' Manual 1.0 > EXTAL is listed as 27MHz in table 8.4e but 25MHz (below) table 8.2e. Nice catch: - Divider 24 is correct for 25 MHz, but not for 27 Mhz. - Multipliers 148 and 118 are correct for 27 MHz, but not for 25 MHz. Needs clarification... 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 07/11] clk: renesas: rcar-gen3: Add support for RCKSEL clock selection
On Wed, Jul 11, 2018 at 04:20:56PM +0200, Geert Uytterhoeven wrote: > Add a clock type and macro for defining clocks where the parent and > divider are selected based on the value of the RCKCR.CKSEL bit. > > Signed-off-by: Geert Uytterhoeven > --- > drivers/clk/renesas/rcar-gen3-cpg.c | 23 --- > drivers/clk/renesas/rcar-gen3-cpg.h | 8 +++- > 2 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c > b/drivers/clk/renesas/rcar-gen3-cpg.c > index 7533a51c679bfd54..19a77822757d29de 100644 > --- a/drivers/clk/renesas/rcar-gen3-cpg.c > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c > @@ -1,7 +1,7 @@ > /* > * R-Car Gen3 Clock Pulse Generator > * > - * Copyright (C) 2015-2016 Glider bvba > + * Copyright (C) 2015-2018 Glider bvba > * > * Based on clk-rcar-gen3.c > * > @@ -31,6 +31,8 @@ > #define CPG_PLL2CR 0x002c > #define CPG_PLL4CR 0x01f4 > > +#define CPG_RCKCR_CKSEL BIT(15) /* RCLK Clock Source Select */ > + > struct cpg_simple_notifier { > struct notifier_block nb; > void __iomem *reg; > @@ -444,7 +446,7 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct > device *dev, > unsigned int div = 1; > u32 value; > > - parent = clks[core->parent & 0x]; /* CLK_TYPE_PE uses high bits */ > + parent = clks[core->parent & 0x]; /* some types use high bits */ > if (IS_ERR(parent)) > return ERR_CAST(parent); > > @@ -524,7 +526,7 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct > device *dev, > > if (clk_get_rate(clks[cpg_clk_extalr])) { > parent = clks[cpg_clk_extalr]; > - value |= BIT(15); > + value |= CPG_RCKCR_CKSEL; > } > > writel(value, csn->reg); > @@ -570,6 +572,21 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct > device *dev, > div = cpg_pll_config->osc_prediv * core->div; > break; > > + case CLK_TYPE_GEN3_RCKSEL: > + /* > + * Clock selectable between two parents and two fixed dividers > + * using RCKCR.CKSEL > + */ > + if (readl(base + CPG_RCKCR) & CPG_RCKCR_CKSEL) { > + div = core->div & 0x; > + } else { > + parent = clks[core->parent >> 16]; > + if (IS_ERR(parent)) > + return ERR_CAST(parent); > + div = core->div >> 16; > + } > + break; > + > default: > return ERR_PTR(-EINVAL); > } > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h > b/drivers/clk/renesas/rcar-gen3-cpg.h > index d7d84d9e4a1c9c8b..214df713ce719991 100644 > --- a/drivers/clk/renesas/rcar-gen3-cpg.h > +++ b/drivers/clk/renesas/rcar-gen3-cpg.h > @@ -1,7 +1,7 @@ > /* > * R-Car Gen3 Clock Pulse Generator > * > - * Copyright (C) 2015-2016 Glider bvba > + * Copyright (C) 2015-2018 Glider bvba > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -24,6 +24,7 @@ enum rcar_gen3_clk_types { > CLK_TYPE_GEN3_Z, > CLK_TYPE_GEN3_Z2, > CLK_TYPE_GEN3_OSC, /* OSC EXTAL predivider and fixed divider */ > + CLK_TYPE_GEN3_RCKSEL, /* Select parent/divider using RCKCR.CKSEL */ > }; > > #define DEF_GEN3_SD(_name, _id, _parent, _offset)\ > @@ -37,6 +38,11 @@ enum rcar_gen3_clk_types { > #define DEF_GEN3_OSC(_name, _id, _parent, _div) \ > DEF_BASE(_name, _id, CLK_TYPE_GEN3_OSC, _parent, .div = _div) > > +#define DEF_GEN3_RCKSEL(_name, _id, _parent0, _div0, _parent1, _div1) \ > + DEF_BASE(_name, _id, CLK_TYPE_GEN3_RCKSEL, \ > + (_parent0) << 16 | (_parent1), .div = (_div0) << 16 | (_div1)) > + > + There seems to be an extra blank line here. > struct rcar_gen3_cpg_pll_config { > u8 extal_div; > u8 pll1_mult; > -- > 2.17.1 >
Re: [PATCH 05/11] clk: renesas: r8a77980: Add OSC predivider configuration and clock
On Wed, Jul 11, 2018 at 04:20:54PM +0200, Geert Uytterhoeven wrote: > R-Car Gen3 Hardware Manual Rev.0.54 documents the relation between the > MD13 and MD14 mode pins, and the OSC EXTAL predivider, as used by the > OSC clock. Hence augment the configuration structure with all > documented predivider values. > > Add the OSC clock using the configured predivider. > > Signed-off-by: Geert Uytterhoeven > --- > drivers/clk/renesas/r8a77980-cpg-mssr.c | 24 +--- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/clk/renesas/r8a77980-cpg-mssr.c > b/drivers/clk/renesas/r8a77980-cpg-mssr.c > index d7ebd9ec00594fc8..093677b8927800fa 100644 > --- a/drivers/clk/renesas/r8a77980-cpg-mssr.c > +++ b/drivers/clk/renesas/r8a77980-cpg-mssr.c > @@ -96,6 +96,8 @@ static const struct cpg_core_clk r8a77980_core_clks[] > __initconst = { > DEF_DIV6P1("canfd", R8A77980_CLK_CANFD, CLK_PLL1_DIV4, 0x244), > DEF_DIV6P1("csi0", R8A77980_CLK_CSI0, CLK_PLL1_DIV4, 0x00c), > DEF_DIV6P1("mso", R8A77980_CLK_MSO, CLK_PLL1_DIV4, 0x014), > + > + DEF_GEN3_OSC("osc", R8A77980_CLK_OSC, CLK_EXTAL, 8), > }; > > static const struct mssr_mod_clk r8a77980_mod_clks[] __initconst = { > @@ -171,23 +173,23 @@ static const unsigned int r8a77980_crit_mod_clks[] > __initconst = { > */ > > /* > - * MD EXTAL PLL2PLL1PLL3 > + * MD EXTAL PLL2PLL1PLL3OSC > * 14 13 (MHz) > - * -- > - * 0 0 16.66 x 1 x240x192x192 > - * 0 1 20x 1 x200x160x160 > - * 1 0 27x 1 x148x118x118 > - * 1 1 33.33 / 2 x240x192x192 > + * > + * 0 0 16.66 x 1 x240x192x192/16 > + * 0 1 20x 1 x200x160x160/19 > + * 1 0 27x 1 x148x118x118/24 Not related to this patch, but in the Users' Manual 1.0 EXTAL is listed as 27MHz in table 8.4e but 25MHz (below) table 8.2e. > + * 1 1 33.33 / 2 x240x192x192/32 > */ > #define CPG_PLL_CONFIG_INDEX(md) md) & BIT(14)) >> 13) | \ >(((md) & BIT(13)) >> 13)) > > static const struct rcar_gen3_cpg_pll_config cpg_pll_configs[4] __initconst > = { > - /* EXTAL divPLL1 mult/div PLL3 mult/div */ > - { 1,192,1, 192,1, }, > - { 1,160,1, 160,1, }, > - { 1,118,1, 118,1, }, > - { 2,192,1, 192,1, }, > + /* EXTAL divPLL1 mult/div PLL3 mult/div OSC prediv */ > + { 1,192,1, 192,1, 16, }, > + { 1,160,1, 160,1, 19, }, > + { 1,118,1, 118,1, 24, }, > + { 2,192,1, 192,1, 32, }, > }; > > static int __init r8a77980_cpg_mssr_init(struct device *dev) > -- > 2.17.1 >
Re: [PATCH 2/2] dt-bindings: arm: Document RZ/A2 SoC DT bindings
Hi Chris, On Thu, Jul 12, 2018 at 3:15 PM Chris Brandt wrote: > On Thursday, July 12, 2018, Geert Uytterhoeven wrote: > > > + - RZ/A2 (R7S9210) > > > +compatible = "renesas,r7s9210" > > > > There seems to be a difference between the r7s92104x and the r7s92105x > > parts (with "x" just denoting a different packaging)? > > Do we need one more digit? > > From an "architecture" standpoint all the hardware in the RZ/A2 > (R7S9210xx series) will be the same. So from a device driver standpoint, > CONFIG_ARCH_R7S9210 would cover everything. > > The rest of the numbers are just for package and number of HW channels. > > Of course, sometimes when they make smaller packages, they also make > smaller silicon to make it cheaper. But in that case, they just simply > remove HW or the number of channels for the hardware. (you don't need as > many peripherals if you don't have as many pins anymore). But, they never > change the functionality of the hardware. > > Take for example RZ/A1 > RZ/A1H R7S72100x > RZ/A1M R7S72101x > RZ/A1L R7S72102x > RZ/A1LU R7S72103x > > These parts all had the same hardware, but different package options And > the "L" parts were cheaper because they reduced the die size by > removing HW. > But the same drivers worked on all of them because the IP was all > exactly the same. > So I would have suggested CONFIG_ARCH_R7S7210 for the RZ/A1 series. > (well, until I found about the R-Car part that took the same part number in > this series) That's the issue with using wildcards, or truncating part numbers: you don't know what unrelated future parts may match... > As for the r7s92104x vs r7s92105x, that is for a HW feature that will > have nothing to do with Linux, so we can ignore that number. OK. > But even if we did make a tiny cut-down version of the device, say a > R7S92106x, all HW IP would be the same, just less of it. So in my mind, the > architecture (from a CONFIG_ARCH perspective) is still the same. Maybe > just a different .dtsi. > > What do you think? As the IP cores are the same in all variants, using "renesas,r7s9210-" should be fine for matching drivers to IP cores. Same for CONFIG_ARCH_R7S9210. However, as the actual dies differ between H, M, and L versions, there may be integration issues to be worked around. So I think it would be wise to use one more digit in the compatible value at the main SoC level, i.e. "renesas,r7s92104". Unless there's a hardware register to detect the version at runtime. But it seems RZ/A2 doesn't have a Product Register (PRR), which most other SH/R-Mobile and R-Car SoCs do have? Thanks! 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 2/2] dt-bindings: arm: Document RZ/A2 SoC DT bindings
Hi Geert, On Thursday, July 12, 2018, Geert Uytterhoeven wrote: > > + - RZ/A2 (R7S9210) > > +compatible = "renesas,r7s9210" > > There seems to be a difference between the r7s92104x and the r7s92105x > parts (with "x" just denoting a different packaging)? > Do we need one more digit? From an "architecture" standpoint all the hardware in the RZ/A2 (R7S9210xx series) will be the same. So from a device driver standpoint, CONFIG_ARCH_R7S9210 would cover everything. The rest of the numbers are just for package and number of HW channels. Of course, sometimes when they make smaller packages, they also make smaller silicon to make it cheaper. But in that case, they just simply remove HW or the number of channels for the hardware. (you don't need as many peripherals if you don't have as many pins anymore). But, they never change the functionality of the hardware. Take for example RZ/A1 RZ/A1H R7S72100x RZ/A1M R7S72101x RZ/A1L R7S72102x RZ/A1LU R7S72103x These parts all had the same hardware, but different package options And the "L" parts were cheaper because they reduced the die size by removing HW. But the same drivers worked on all of them because the IP was all exactly the same. So I would have suggested CONFIG_ARCH_R7S7210 for the RZ/A1 series. (well, until I found about the R-Car part that took the same part number in this series) As for the r7s92104x vs r7s92105x, that is for a HW feature that will have nothing to do with Linux, so we can ignore that number. But even if we did make a tiny cut-down version of the device, say a R7S92106x, all HW IP would be the same, just less of it. So in my mind, the architecture (from a CONFIG_ARCH perspective) is still the same. Maybe just a different .dtsi. What do you think? Chris
Re: [PATCH v2 4/4] mmc: renesas_sdhi: skip SCC error check when retuning
On Thu, Jul 05, 2018 at 04:18:41PM +0200, Niklas Söderlund wrote: > From: Masaharu Hayakawa > > Checking for SCC error during retuning is unnecessary. > > Signed-off-by: Masaharu Hayakawa > [Niklas: fix small style issue] > Signed-off-by: Niklas Söderlund > --- > drivers/mmc/host/renesas_sdhi_core.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c > b/drivers/mmc/host/renesas_sdhi_core.c > index 589da18bd636c2f4..ef711c532a26babe 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -445,6 +445,13 @@ static bool renesas_sdhi_check_scc_error(struct > tmio_mmc_host *host) > { > struct renesas_sdhi *priv = host_to_priv(host); > > + if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) && > + !(host->mmc->ios.timing == MMC_TIMING_MMC_HS200)) > + return false; > + > + if (host->mmc->doing_retune) > + return false; > + I believe this patch needs to be updaed now that HS400 support has been merged. > /* Check SCC error */ > if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) & > SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN && > -- > 2.17.0 >
Re: [PATCH v2 2/4] mmc: tmio: Fix SCC error detection
On Thu, Jul 05, 2018 at 04:18:39PM +0200, Niklas Söderlund wrote: > From: Masaharu Hayakawa > > SDR104 and HS200 need to check for SCC error. If SCC error is detected, > retuning is necessary. > > Signed-off-by: Masaharu Hayakawa > [Niklas: update commit message] > Signed-off-by: Niklas Söderlund Reviewed-by: Simon Horman > --- > drivers/mmc/host/tmio_mmc_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc_core.c > b/drivers/mmc/host/tmio_mmc_core.c > index 000fa9dff784ecb0..525f16f4e33b01ae 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -919,8 +919,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host > *host) > if (mrq->cmd->error || (mrq->data && mrq->data->error)) > tmio_mmc_abort_dma(host); > > - if (host->check_scc_error) > - host->check_scc_error(host); > + if (host->check_scc_error && host->check_scc_error(host)) > + mrq->cmd->error = -EILSEQ; > > /* If SET_BLOCK_COUNT, continue with main command */ > if (host->mrq && !mrq->cmd->error) { > -- > 2.17.0 >
Re: [PATCH 1/2] ARM: shmobile: Add basic RZ/A2 SoC support
Hi Chris, On Thu, Jul 12, 2018 at 5:02 AM Chris Brandt wrote: > Add the RZ/A2 SoC to the Renesas SoC collection. > > Signed-off-by: Chris Brandt > --- /dev/null > +++ b/arch/arm/mach-shmobile/setup-r7s9210.c > @@ -0,0 +1,27 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * r7s9210 processor support > + * > + * Copyright (C) 2018 Renesas Electronics Corporation > + * Copyright (C) 2018 Chris Brandt > + * > + */ > + > +#include > + > +#include > + > +#include "common.h" > + > +static const char *const r7s9210_boards_compat_dt[] __initconst = { > + "renesas,r7s9210", > + NULL, > +}; > + > +DT_MACHINE_START(R7S72100_DT, "Generic R7S9210 (Flattened Device Tree)") > + .l2c_aux_val= 0, > + .l2c_aux_mask = ~0, > + .init_early = shmobile_init_delay, > + .init_late = shmobile_init_late, > + .dt_compat = r7s9210_boards_compat_dt, > +MACHINE_END I'm wondering if you could do without any board code, i.e. without setup-r7s9210.c? 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 2/2] dt-bindings: arm: Document RZ/A2 SoC DT bindings
Hi Chris, On Thu, Jul 12, 2018 at 5:02 AM Chris Brandt wrote: > Add device tree bindings documentation for Renesas RZ/A2 (r7s9210) SoC. > > Signed-off-by: Chris Brandt Reviewed-by: Geert Uytterhoeven > --- a/Documentation/devicetree/bindings/arm/shmobile.txt > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt > @@ -7,6 +7,8 @@ SoCs: > compatible = "renesas,emev2" >- RZ/A1H (R7S72100) > compatible = "renesas,r7s72100" > + - RZ/A2 (R7S9210) > +compatible = "renesas,r7s9210" There seems to be a difference between the r7s92104x and the r7s92105x parts (with "x" just denoting a different packaging)? Do we need one more digit? >- SH-Mobile AG5 (R8A73A00/SH73A0) > compatible = "renesas,sh73a0" >- R-Mobile APE6 (R8A73A40) 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 2/2] serial: 8250_dw: Add compatible string for Renesas RZ/N1 UART
Hi Phil, On Thu, Jul 12, 2018 at 2:03 PM Phil Edworthy wrote: > On 11 July 2018 16:02, Phil Edworthy wrote: > > On 11 July 2018 13:42, Geert Uytterhoeven wrote: > > > On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy wrote: > > > > The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has > > > > additional registers for DMA. This patch does not address the > > > > changes required for DMA support, it simply adds the compatible string. > > > > > > > > Signed-off-by: Phil Edworthy > > > > > > Thanks for your patch! > > > > [...] > > > > --- a/drivers/tty/serial/8250/8250_dw.c > > > > +++ b/drivers/tty/serial/8250/8250_dw.c > > > > @@ -693,6 +693,7 @@ static const struct of_device_id > > > > dw8250_of_match[] > > > = { > > > > { .compatible = "snps,dw-apb-uart" }, > > > > { .compatible = "cavium,octeon-3860-uart" }, > > > > { .compatible = "marvell,armada-38x-uart" }, > > > > + { .compatible = "renesas,uart-rzn1" }, > > > > > > renesas,rzn1-uart > I missed this comment. I followed the renesas convention of > renesas,peripheral-device > that is used for most R-Car drivers, but happy to change. "renesas,-" is the old scheme, which we cannot change for existing devices due to backwards compatibility. "renesas,-" is the "new" scheme, consistent with other vendors. 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 1/2] dt: serial: Add Renesas RZ/N1 binding documentation
Hi Geert, On 11 July 2018 13:39, Geert Uytterhoeven wrote: > On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy wrote: > > The RZ/N1 UART is a modified Synopsys DesignWare UART. > > The modifications only relate to DMA so you could actually use the > > controller with the Synopsys compatible string if you are not using > > DMA, but you should not do so. > > > > Signed-off-by: Phil Edworthy > > Thanks for your patch! > > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/serial/renesas,rzn1-uart.txt > > @@ -0,0 +1,10 @@ > > +Renesas RZ/N1 UART > > + > > +This controller is based on the Synopsys DesignWare ABP UART and > > +inherits all properties defined in snps-dw-apb-uart.txt except for the > compatible property. > > + > > +Required properties: > > +- compatible : The device specific string followed by the generic RZ/N1 > string. > > + Therefore it must be one of: > > + "renesas,uart-r9a06g032", "renesas,uart-rzn1" > > "renesas,r9a06g032-uart", "renesas,rzn1-uart" > > > + "renesas,uart-r9a06g033", "renesas,uart-rzn1" > > "renesas,r9a06g033-uart", "renesas,rzn1-uart" I followed the renesas convention of renesas,peripheral-device that is used for most R-Car drivers, but happy to change. > I assume you plan to describe the DMA-related properties later? Yes, it will take a while to sort out. Thanks Phil
RE: [PATCH 2/2] serial: 8250_dw: Add compatible string for Renesas RZ/N1 UART
Hi Geert, On 11 July 2018 16:02, Phil Edworthy wrote: > On 11 July 2018 13:42, Geert Uytterhoeven wrote: > > On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy wrote: > > > The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has > > > additional registers for DMA. This patch does not address the > > > changes required for DMA support, it simply adds the compatible string. > > > > > > Signed-off-by: Phil Edworthy > > > > Thanks for your patch! > > [...] > > > --- a/drivers/tty/serial/8250/8250_dw.c > > > +++ b/drivers/tty/serial/8250/8250_dw.c > > > @@ -693,6 +693,7 @@ static const struct of_device_id > > > dw8250_of_match[] > > = { > > > { .compatible = "snps,dw-apb-uart" }, > > > { .compatible = "cavium,octeon-3860-uart" }, > > > { .compatible = "marvell,armada-38x-uart" }, > > > + { .compatible = "renesas,uart-rzn1" }, > > > > renesas,rzn1-uart I missed this comment. I followed the renesas convention of renesas,peripheral-device that is used for most R-Car drivers, but happy to change. Thanks Phil
[PATCH] gpio: rcar: Implement .get_direction() callback
Allow gpiolib to read back the current I/O direction configuration by implementing the .get_direction() callback. Signed-off-by: Geert Uytterhoeven --- drivers/gpio/gpio-rcar.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c index 4d2cc6d32184f390..ec04752b6bd25fb6 100644 --- a/drivers/gpio/gpio-rcar.c +++ b/drivers/gpio/gpio-rcar.c @@ -278,6 +278,13 @@ static void gpio_rcar_free(struct gpio_chip *chip, unsigned offset) pm_runtime_put(>pdev->dev); } +static int gpio_rcar_get_direction(struct gpio_chip *chip, unsigned int offset) +{ + struct gpio_rcar_priv *p = gpiochip_get_data(chip); + + return !(gpio_rcar_read(p, INOUTSEL) & BIT(offset)); +} + static int gpio_rcar_direction_input(struct gpio_chip *chip, unsigned offset) { gpio_rcar_config_general_input_output_mode(chip, offset, false); @@ -461,6 +468,7 @@ static int gpio_rcar_probe(struct platform_device *pdev) gpio_chip = >gpio_chip; gpio_chip->request = gpio_rcar_request; gpio_chip->free = gpio_rcar_free; + gpio_chip->get_direction = gpio_rcar_get_direction; gpio_chip->direction_input = gpio_rcar_direction_input; gpio_chip->get = gpio_rcar_get; gpio_chip->direction_output = gpio_rcar_direction_output; -- 2.17.1
Re: [PATCH 0/2] serial: sh-sci: Use pm_runtime_get_sync()/put()
On Fri, Apr 13, 2018 at 07:27:59PM +0200, Geert Uytterhoeven wrote: > Hi Ulrich, > > On Fri, Apr 13, 2018 at 7:00 PM, Ulrich Hecht > wrote: > > These patches make sure that the device is up while the suspend/resume code > > is executed. Up-port from the BSP, tested not to break stuff. > > > > Hien Dang (2): > > serial: sh-sci: Use pm_runtime_get_sync()/put() on suspend > > serial: sh-sci: Use pm_runtime_get_sync()/put() on resume > > I don't think it makes much sense to split this in two parts... > > Furthermore, shouldn't this be handled by the serial core, calling > uart_change_pm()? > > It looks like uart_resume_port() already changes the port's state to > enabled, while uart_suspend_port() assumes the port is already enabled, > and disables it. > > Perhaps handling is not correct for some code paths? Ulrich, what is your take on this? signature.asc Description: PGP signature