LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Peter Griffin <peter.griffin@linaro.org>
To: Lee Jones <lee.jones@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	computersforpeace@gmail.com, kernel@stlinux.com
Subject: Re: [STLinux Kernel] [PATCH v2 RESEND 02/11] mtd: st_spi_fsm: Obtain and use EMI clock if provided
Date: Mon, 8 Dec 2014 13:52:23 +0000	[thread overview]
Message-ID: <20141208135223.GB29220@griffinp-ThinkPad-X1-Carbon-2nd> (raw)
In-Reply-To: <1417688512-7644-3-git-send-email-lee.jones@linaro.org>

Hi Lee,

On Thu, 04 Dec 2014, Lee Jones wrote:

> ST's Common Clk Framework is now available. This patch ensures the FSM
> makes use of it by obtaining and enabling the EMI clock if provided. If
> system fails to provide the EMI clock FSM uses its original default
> rate.

<snip>

I'm not sure I understand this patch. Now that common clock framework
is available for STI platforms, why would the emi clock ever not be available?

If it is to maintain DT compatability then we are already breaking this with the
syscfg 'reg' DT changes which are coming up, and this was deemed OK as the platform
is WIP.

IMO keeping this legacy code which assumes the bootloader / JTAG has enabled and configured
the emi clock correctly is not worth it now that CCF is available for the platform.

> -	/* TODO: Make this dynamic */
> -	emi_freq = STFSM_DEFAULT_EMI_FREQ;
> +	if (!fsm->clk) {
> +		dev_warn(fsm->dev,
> +			 "No EMI clock available. Using default 100MHz.\n");
> +
> +		emi_freq = STFSM_DEFAULT_EMI_FREQ;
> +	} else
> +		emi_freq = clk_get_rate(fsm->clk);
>  
>  	/*
>  	 * Calculate clk_div - values between 2 and 128
> @@ -2057,6 +2064,15 @@ static int stfsm_probe(struct platform_device *pdev)
>  		return PTR_ERR(fsm->base);
>  	}
>  
> +	fsm->clk = devm_clk_get(&pdev->dev, "emi_clk");
> +	if (IS_ERR(fsm->clk)) {
> +		dev_warn(fsm->dev, "Couldn't find EMI clock.\n");
> +		fsm->clk = NULL;
> +	} else if (clk_prepare_enable(fsm->clk)) {
> +		dev_warn(fsm->dev, "Failed to enable EMI clock.\n");

If a clock has been provided and we fail to enable it, then surely that is game over?

As the code currently is it then goes onto assume in stfsm_set_freq() that the rate
is STFSM_DEFAULT_EMI_FREQ which can't be true if it can't enable the clock.

regards,

Peter.

  reply	other threads:[~2014-12-08 13:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-04 10:21 [PATCH v2 RESEND 00/11] mtd: st_spi_fsm: Align with ST's internal development Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 01/11] mtd: st_spi_fsm: Extend fsm_clear_fifo to handle unwanted bytes Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 02/11] mtd: st_spi_fsm: Obtain and use EMI clock if provided Lee Jones
2014-12-08 13:52   ` Peter Griffin [this message]
2014-12-08 13:57     ` [STLinux Kernel] " Peter Griffin
2014-12-04 10:21 ` [PATCH v2 RESEND 03/11] mtd: st_spi_fsm: Fix [-Wsign-compare] build warning Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 04/11] mtd: st_spi_fsm: Add support for Micron N25Q512A Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 05/11] mtd: st_spi_fsm: Add support for N25Q512 and N25Q00A devices Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 06/11] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 07/11] mtd: st_spi_fsm: Update Spansion device entries Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 08/11] mtd: st_spi_fsm: Improve busy wait handling Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 09/11] mtd: st_spi_fsm: Provide documentation for boot device mask property Lee Jones
2014-12-04 10:21 ` [PATCH v2 RESEND 10/11] mtd: st_spi_fsm: Provide mask to obtain correct boot device pins Lee Jones
2014-12-08 14:43   ` [STLinux Kernel] " Peter Griffin
2014-12-04 10:21 ` [PATCH v2 RESEND 11/11] mtd: st_spi_fsm: General tidy-up Lee Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141208135223.GB29220@griffinp-ThinkPad-X1-Carbon-2nd \
    --to=peter.griffin@linaro.org \
    --cc=computersforpeace@gmail.com \
    --cc=kernel@stlinux.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --subject='Re: [STLinux Kernel] [PATCH v2 RESEND 02/11] mtd: st_spi_fsm: Obtain and use EMI clock if provided' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).