Re: [PATCH -next] xtensa: Fix build error due to missing include file

[Guenter Roeck]

On Mon, Jul 25, 2016 at 11:07:00AM +0300, Max Filippov wrote:
> On Sun, Jul 24, 2016 at 7:27 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > On 07/24/2016 08:18 AM, Rob Herring wrote:
> >>
> >> On Sun, Jul 24, 2016 at 9:20 AM, Max Filippov <jcmvbkbc@xxxxxxxxx> wrote:
> >>>
> >>> On Sun, Jul 24, 2016 at 9:00 AM, Guenter Roeck <linux@xxxxxxxxxxxx>
> >>> wrote:
> >>>>
> >>>> On 07/23/2016 05:45 PM, Max Filippov wrote:
> >>>>>
> >>>>>
> >>>>> On Sun, Jul 24, 2016 at 3:24 AM, Guenter Roeck <linux@xxxxxxxxxxxx>
> >>>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Commit 69d99e6c0d621f ("xtensa: Remove unnecessary
> >>>>>> of_platform_populate
> >>>>>> with default match table") dropped various include files from
> >>>>>> arch/xtensa/kernel/setup.c. This results in the following build error.
> >>>>>>
> >>>>>> arch/xtensa/kernel/setup.c: In function âxtensa_dt_io_areaâ:
> >>>>>> arch/xtensa/kernel/setup.c:213:2: error:
> >>>>>>           implicit declaration of function âof_read_ulongâ
> >>>>>>
> >>>>>> Fixes: 69d99e6c0d621f ("xtensa: Remove unnecessary
> >>>>>> of_platform_populate
> >>>>>> with default match table")
> >>>>>> Cc: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> >>>>>> Cc: Rob Herring <robh@xxxxxxxxxx>
> >>>>>> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> >>>>>> ---
> >>>>>> I would suggest to merge this patch into the commit causing the
> >>>>>> problem
> >>>>>> if possible.
> >>>>>>
> >>>>>>    arch/xtensa/kernel/setup.c | 1 +
> >>>>>>    1 file changed, 1 insertion(+)
> >>>>>
> >>>>>
> >>>>>
> >>>>> Acked-by: Max Filippov <jcmvbkbc@xxxxxxxxx>
> >>>>>
> >>>>
> >>>> Heads up: xtensa images build after this patch, but the image that
> >>>> previously failed to build (generic_kc705_defconfig) doesn't boot,
> >>>> at least not with qemu. I don't know if this is caused by 69d99e6c0d621f
> >>>> or by some other patch.
> >>>
> >>>
> >>> It seems to be booting, but serial console is non-functional.
> >>> With commit 69d99e6c0d621f ("xtensa: Remove unnecessary
> >>> of_platform_populate
> >>> with default match table") I see the following message during boot:
> >>>
> >>> [    6.375284] of_serial fd050020.serial: clk or clock-frequency not
> >>> defined
> >>>
> >>> Reverting that commit fixes serial console for me.
> >>> Any suggestions on what might go wrong?
> >>
> >>
> >> Probably the init order changed somehow and deferred probe is not
> >> supported correctly.
> >>
> >
> > While the message is due to -EPROBE_DEFER (and there should really be no
> > message
> > in this case), I suspect that the underlying problem is that of_clk_init()
> > is
> > now called way too early, before the clock source is fully initialized.
> > Specifically, the call now happens before the message "clocksource: Switched
> > to clocksource ccount", which at least looks suspicious.
> >
> > If I move of_clk_init() back into xtensa_device_probe(), to be called as
> > device_initcall(), everything is fine.
> 
> The problem is in the way the main clock on xtfpga platform is set up:
> it is done by updating corresponding OF node's clock-frequency attribute
> and that must happen before the of_clk_init call.
> Looks like it's time to create a clock provider for xtfpga boards, something
> like the one attached.
> 
> -- 
> Thanks.
> -- Max

> From 8aa959869060cb5ffbc06c4c25cfb0e6d81bc8b4 Mon Sep 17 00:00:00 2001
> From: Max Filippov <jcmvbkbc@xxxxxxxxx>
> Date: Mon, 25 Jul 2016 10:58:10 +0300
> Subject: [PATCH] xtensa: xtfpga: add clock provider
> 
> Signed-off-by: Max Filippov <jcmvbkbc@xxxxxxxxx>

In case you want to apply this patch:

Tested-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>  arch/xtensa/boot/dts/xtfpga.dtsi     | 11 ++++++-----
>  arch/xtensa/platforms/xtfpga/setup.c | 32 ++++++++++++++++++++++++++++----
>  2 files changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/xtensa/boot/dts/xtfpga.dtsi b/arch/xtensa/boot/dts/xtfpga.dtsi
> index cd45f9c..b49b5d4 100644
> --- a/arch/xtensa/boot/dts/xtfpga.dtsi
> +++ b/arch/xtensa/boot/dts/xtfpga.dtsi
> @@ -36,11 +36,6 @@
>  	};
>  
>  	clocks {
> -		osc: main-oscillator {
> -			#clock-cells = <0>;
> -			compatible = "fixed-clock";
> -		};
> -
>  		clk54: clk54 {
>  			#clock-cells = <0>;
>  			compatible = "fixed-clock";
> @@ -54,6 +49,12 @@
>  		compatible = "simple-bus";
>  		ranges = <0x00000000 0xf0000000 0x10000000>;
>  
> +		osc: main-oscillator {
> +			reg = <0x0d020004 0x4>;
> +			#clock-cells = <0>;
> +			compatible = "cdns,xtfpga-clock";
> +		};
> +
>  		serial0: serial@0d050020 {
>  			device_type = "serial";
>  			compatible = "ns16550a";
> diff --git a/arch/xtensa/platforms/xtfpga/setup.c b/arch/xtensa/platforms/xtfpga/setup.c
> index b509d1f..24f22e8 100644
> --- a/arch/xtensa/platforms/xtfpga/setup.c
> +++ b/arch/xtensa/platforms/xtfpga/setup.c
> @@ -26,6 +26,8 @@
>  #include <linux/console.h>
>  #include <linux/delay.h>
>  #include <linux/of.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
>  
>  #include <asm/timex.h>
>  #include <asm/processor.h>
> @@ -135,14 +137,36 @@ static void __init update_local_mac(struct device_node *node)
>  	of_update_property(node, newmac);
>  }
>  
> +static void __init xtfpga_clk_setup(struct device_node *np)
> +{
> +	void __iomem *base = of_iomap(np, 0);
> +	struct clk *clk;
> +	u32 freq;
> +
> +	if (!base) {
> +		pr_err("%s: invalid address\n", np->name);
> +		return;
> +	}
> +
> +	freq = __raw_readl(base);
> +	clk = clk_register_fixed_rate(NULL, np->name, NULL, 0, freq);
> +
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: clk registration failed\n", np->name);
> +		return;
> +	}
> +
> +	if (of_clk_add_provider(np, of_clk_src_simple_get, clk)) {
> +		pr_err("%s: clk provider registration failed\n", np->name);
> +		return;
> +	}
> +}
> +CLK_OF_DECLARE(xtfpga_clk, "cdns,xtfpga-clock", xtfpga_clk_setup);
> +
>  static int __init machine_setup(void)
>  {
> -	struct device_node *clock;
>  	struct device_node *eth = NULL;
>  
> -	for_each_node_by_name(clock, "main-oscillator")
> -		update_clock_frequency(clock);
> -
>  	if ((eth = of_find_compatible_node(eth, NULL, "opencores,ethoc")))
>  		update_local_mac(eth);
>  	return 0;
> -- 
> 2.1.4
>