| Message ID | 1461823717-22517-1-git-send-email-dirk.behme@de.bosch.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Dirk, On Thu, Apr 28, 2016 at 8:08 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote: > From: Oleksij Rempel <linux@rempel-privat.de> > > Map the Mode Monitor Register via the device tree instead of > hard coding it in the code. This makes the mapping known to the > device tree. > > Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com> > Signed-off-by: Oleksij Rempel <linux@rempel-privat.de> > Signed-off-by: Oleksij Rempel <fixed-term.Oleksij.Rempel@de.bosch.com> > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> > --- > .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 9 ++++++--- > arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 +++- > drivers/clk/renesas/r8a7795-cpg-mssr.c | 22 +++------------------- > drivers/clk/renesas/renesas-cpg-mssr.c | 19 ++++++++++++++----- > drivers/clk/renesas/renesas-cpg-mssr.h | 2 +- > 5 files changed, 27 insertions(+), 29 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > index fefb802..7984485 100644 > --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > @@ -15,8 +15,10 @@ Required Properties: > - compatible: Must be one of: > - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC > > - - reg: Base address and length of the memory resource used by the CPG/MSSR > - block > + - reg: > + - 0: Base address and length of the memory resource used by the CPG/MSSR > + block. > + - 1: Mode Monitor Register. > > - clocks: References to external parent clocks, one entry for each entry in > clock-names > @@ -46,7 +48,8 @@ Examples > > cpg: clock-controller@e6150000 { > compatible = "renesas,r8a7795-cpg-mssr"; > - reg = <0 0xe6150000 0 0x1000>; > + reg = <0 0xe6150000 0 0x1000>, > + <0 0xe6160060 0 0x4>; The MODEMR register is not part of the CPG block, but of the RST block. So IMHO it doesn't belong in the clock-controller node. BTW, I still prefer something like "[PATCH 0/6] arm64: renesas: Obtain MD pin values using syscon/regmap" (http://www.spinics.net/lists/linux-sh/msg44757.html). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Hi Geert, On 28.04.2016 08:52, Geert Uytterhoeven wrote: > Hi Dirk, > > On Thu, Apr 28, 2016 at 8:08 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote: >> From: Oleksij Rempel <linux@rempel-privat.de> >> >> Map the Mode Monitor Register via the device tree instead of >> hard coding it in the code. This makes the mapping known to the >> device tree. >> >> Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com> >> Signed-off-by: Oleksij Rempel <linux@rempel-privat.de> >> Signed-off-by: Oleksij Rempel <fixed-term.Oleksij.Rempel@de.bosch.com> >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> >> --- >> .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 9 ++++++--- >> arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 +++- >> drivers/clk/renesas/r8a7795-cpg-mssr.c | 22 +++------------------- >> drivers/clk/renesas/renesas-cpg-mssr.c | 19 ++++++++++++++----- >> drivers/clk/renesas/renesas-cpg-mssr.h | 2 +- >> 5 files changed, 27 insertions(+), 29 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt >> index fefb802..7984485 100644 >> --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt >> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt >> @@ -15,8 +15,10 @@ Required Properties: >> - compatible: Must be one of: >> - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC >> >> - - reg: Base address and length of the memory resource used by the CPG/MSSR >> - block >> + - reg: >> + - 0: Base address and length of the memory resource used by the CPG/MSSR >> + block. >> + - 1: Mode Monitor Register. >> >> - clocks: References to external parent clocks, one entry for each entry in >> clock-names >> @@ -46,7 +48,8 @@ Examples >> >> cpg: clock-controller@e6150000 { >> compatible = "renesas,r8a7795-cpg-mssr"; >> - reg = <0 0xe6150000 0 0x1000>; >> + reg = <0 0xe6150000 0 0x1000>, >> + <0 0xe6160060 0 0x4>; > > The MODEMR register is not part of the CPG block, but of the RST block. > So IMHO it doesn't belong in the clock-controller node. Ok, fine. So we agree hat this ugly hard coded register address stuff has to be removed and replaced by a device tree based solution. Correct? > BTW, I still prefer something like > "[PATCH 0/6] arm64: renesas: Obtain MD pin values using syscon/regmap" > (http://www.spinics.net/lists/linux-sh/msg44757.html). What's the status of this? I see there a date "1 Sep 2015" ;) Is this available in any testing tree we could try? Best regards Dirk
Hi Dirk, On Thu, Apr 28, 2016 at 8:58 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote: > On 28.04.2016 08:52, Geert Uytterhoeven wrote: >> On Thu, Apr 28, 2016 at 8:08 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote: >>> @@ -46,7 +48,8 @@ Examples >>> >>> cpg: clock-controller@e6150000 { >>> compatible = "renesas,r8a7795-cpg-mssr"; >>> - reg = <0 0xe6150000 0 0x1000>; >>> + reg = <0 0xe6150000 0 0x1000>, >>> + <0 0xe6160060 0 0x4>; >> >> >> The MODEMR register is not part of the CPG block, but of the RST block. >> So IMHO it doesn't belong in the clock-controller node. > > Ok, fine. > > So we agree hat this ugly hard coded register address stuff has to be > removed and replaced by a device tree based solution. Correct? Yes we agree. >> BTW, I still prefer something like >> "[PATCH 0/6] arm64: renesas: Obtain MD pin values using syscon/regmap" >> (http://www.spinics.net/lists/linux-sh/msg44757.html). > > What's the status of this? I see there a date "1 Sep 2015" ;) We were considering alternative solutions, cfr. "[PATCH/RFC 0/6] boot-mode-reg: Add core" (http://www.spinics.net/lists/linux-sh/msg45969.html). To avoid delaying upstream r8a7795, we settled for the ugly hard coded register access as an interim solution. > Is this available in any testing tree we could try? It used to be part of older renesas-drivers releases. The patches may still apply, though. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Hi Geert, On 28.04.2016 10:07, Geert Uytterhoeven wrote: > Hi Dirk, > > On Thu, Apr 28, 2016 at 8:58 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote: >> On 28.04.2016 08:52, Geert Uytterhoeven wrote: >>> On Thu, Apr 28, 2016 at 8:08 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote: >>>> @@ -46,7 +48,8 @@ Examples >>>> >>>> cpg: clock-controller@e6150000 { >>>> compatible = "renesas,r8a7795-cpg-mssr"; >>>> - reg = <0 0xe6150000 0 0x1000>; >>>> + reg = <0 0xe6150000 0 0x1000>, >>>> + <0 0xe6160060 0 0x4>; >>> >>> >>> The MODEMR register is not part of the CPG block, but of the RST block. >>> So IMHO it doesn't belong in the clock-controller node. >> >> Ok, fine. >> >> So we agree hat this ugly hard coded register address stuff has to be >> removed and replaced by a device tree based solution. Correct? > > Yes we agree. > >>> BTW, I still prefer something like >>> "[PATCH 0/6] arm64: renesas: Obtain MD pin values using syscon/regmap" >>> (http://www.spinics.net/lists/linux-sh/msg44757.html). >> >> What's the status of this? I see there a date "1 Sep 2015" ;) > > We were considering alternative solutions, cfr. "[PATCH/RFC 0/6] boot-mode-reg: > Add core" (http://www.spinics.net/lists/linux-sh/msg45969.html). > To avoid delaying upstream r8a7795, we settled for the ugly hard coded > register access as an interim solution. > >> Is this available in any testing tree we could try? > > It used to be part of older renesas-drivers releases. Would it be possible to add them again then, now? Best regards Dirk
diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt index fefb802..7984485 100644 --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt @@ -15,8 +15,10 @@ Required Properties: - compatible: Must be one of: - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC - - reg: Base address and length of the memory resource used by the CPG/MSSR - block + - reg: + - 0: Base address and length of the memory resource used by the CPG/MSSR + block. + - 1: Mode Monitor Register. - clocks: References to external parent clocks, one entry for each entry in clock-names @@ -46,7 +48,8 @@ Examples cpg: clock-controller@e6150000 { compatible = "renesas,r8a7795-cpg-mssr"; - reg = <0 0xe6150000 0 0x1000>; + reg = <0 0xe6150000 0 0x1000>, + <0 0xe6160060 0 0x4>; clocks = <&extal_clk>, <&extalr_clk>; clock-names = "extal", "extalr"; #clock-cells = <2>; diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index 8be9424..3e5d5b0 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -305,7 +305,9 @@ cpg: clock-controller@e6150000 { compatible = "renesas,r8a7795-cpg-mssr"; - reg = <0 0xe6150000 0 0x1000>; + reg = <0 0xe6150000 0 0x1000>, + /* MODEMR - Mode Monitor Register */ + <0 0xe6160060 0 0x4>; clocks = <&extal_clk>, <&extalr_clk>; clock-names = "extal", "extalr"; #clock-cells = <2>; diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c index 6af7f5b..ac34c11 100644 --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c @@ -612,26 +612,10 @@ struct clk * __init r8a7795_cpg_clk_register(struct device *dev, __clk_get_name(parent), 0, mult, div); } -/* - * Reset register definitions. - */ -#define MODEMR 0xe6160060 - -static u32 rcar_gen3_read_mode_pins(void) -{ - void __iomem *modemr = ioremap_nocache(MODEMR, 4); - u32 mode; - - BUG_ON(!modemr); - mode = ioread32(modemr); - iounmap(modemr); - - return mode; -} - -static int __init r8a7795_cpg_mssr_init(struct device *dev) +static int __init r8a7795_cpg_mssr_init(struct device *dev, + void __iomem *modemr_base) { - u32 cpg_mode = rcar_gen3_read_mode_pins(); + u32 cpg_mode = ioread32(modemr_base); cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; if (!cpg_pll_config->extal_div) { diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c index 1f2dc362..b455b14 100644 --- a/drivers/clk/renesas/renesas-cpg-mssr.c +++ b/drivers/clk/renesas/renesas-cpg-mssr.c @@ -107,6 +107,7 @@ static const u16 srcr[] = { struct cpg_mssr_priv { struct device *dev; void __iomem *base; + void __iomem *modemr_base; /* Mode Monitor Register */ spinlock_t mstp_lock; struct clk **clks; @@ -529,17 +530,25 @@ static int __init cpg_mssr_probe(struct platform_device *pdev) int error; info = of_match_node(cpg_mssr_match, np)->data; - if (info->init) { - error = info->init(dev); - if (error) - return error; - } priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; priv->dev = dev; + + if (info->init) { + /* get Mode Monitor Register */ + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + priv->modemr_base = devm_ioremap_resource(dev, res); + if (IS_ERR(priv->modemr_base)) + return PTR_ERR(priv->modemr_base); + + error = info->init(dev, priv->modemr_base); + if (error) + return error; + } + spin_lock_init(&priv->mstp_lock); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h index 0d1e3e8..3ec7e2d 100644 --- a/drivers/clk/renesas/renesas-cpg-mssr.h +++ b/drivers/clk/renesas/renesas-cpg-mssr.h @@ -123,7 +123,7 @@ struct cpg_mssr_info { unsigned int num_core_pm_clks; /* Callbacks */ - int (*init)(struct device *dev); + int (*init)(struct device *dev, void __iomem *modemr_base); struct clk *(*cpg_clk_register)(struct device *dev, const struct cpg_core_clk *core, const struct cpg_mssr_info *info,