All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Viresh Kumar <viresh.kumar@st.com>
Cc: akpm@linux-foundation.org, spear-devel@list.st.com,
	viresh.linux@gmail.com, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, mturquette@linaro.org,
	sshtylyov@mvista.com, jgarzik@redhat.com, linux@arm.linux.org.uk,
	Andrew Lunn <andrew@lunn.ch>,
	linux-ide@vger.kernel.org
Subject: Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Date: Tue, 24 Apr 2012 14:00:19 +0200	[thread overview]
Message-ID: <20120424120019.GE24089@lunn.ch> (raw)
In-Reply-To: <abbcf661dc58948794839eb0fba1d5a0950ec848.1335266056.git.viresh.kumar@st.com>

> -#if defined(CONFIG_HAVE_CLK)
>  	hpriv->clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(hpriv->clk))
> -		dev_notice(&pdev->dev, "cannot get clkdev\n");
> -	else
> -		clk_enable(hpriv->clk);
> -#endif
> +	if (IS_ERR(hpriv->clk)) {
> +		dev_err(&pdev->dev, "cannot get clkdev\n");
> +		return PTR_ERR(hpriv->clk);
> +	}
> +
> +	clk_enable(hpriv->clk);

Sorry, but still wrong.

The clock is optional. If we can find a clock, turn it on. If not,
keep going....

You patch causes the missing clock to become a fatal error.

This sata_mv exists in multiple forms. It can be part of a SoC. It can
also be on a PCI bus. In the PCI form, it is unlikely to have a clk
which can be controlled. When built into a SoC, namely one of the
Orion family, dove, orion5x, mv78xx0 do not have a clock which can be
controlled. However kirkwood does have a clock.

So, kirkwood will provide a clock and expects that sata_mv will turn
it on. All the other ways of using sata_mv will not provide a clock,
but still expect the driver to be happy.

    Andrew

WARNING: multiple messages have this Message-ID (diff)
From: andrew@lunn.ch (Andrew Lunn)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Date: Tue, 24 Apr 2012 14:00:19 +0200	[thread overview]
Message-ID: <20120424120019.GE24089@lunn.ch> (raw)
In-Reply-To: <abbcf661dc58948794839eb0fba1d5a0950ec848.1335266056.git.viresh.kumar@st.com>

> -#if defined(CONFIG_HAVE_CLK)
>  	hpriv->clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(hpriv->clk))
> -		dev_notice(&pdev->dev, "cannot get clkdev\n");
> -	else
> -		clk_enable(hpriv->clk);
> -#endif
> +	if (IS_ERR(hpriv->clk)) {
> +		dev_err(&pdev->dev, "cannot get clkdev\n");
> +		return PTR_ERR(hpriv->clk);
> +	}
> +
> +	clk_enable(hpriv->clk);

Sorry, but still wrong.

The clock is optional. If we can find a clock, turn it on. If not,
keep going....

You patch causes the missing clock to become a fatal error.

This sata_mv exists in multiple forms. It can be part of a SoC. It can
also be on a PCI bus. In the PCI form, it is unlikely to have a clk
which can be controlled. When built into a SoC, namely one of the
Orion family, dove, orion5x, mv78xx0 do not have a clock which can be
controlled. However kirkwood does have a clock.

So, kirkwood will provide a clock and expects that sata_mv will turn
it on. All the other ways of using sata_mv will not provide a clock,
but still expect the driver to be happy.

    Andrew

  reply	other threads:[~2012-04-24 11:58 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-24 11:21 [PATCH V3 00/12] clk: Add non CONFIG_HAVE_CLK routines Viresh Kumar
2012-04-24 11:21 ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 01/12] " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 02/12] clk: Remove redundant depends on from drivers/Kconfig Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 03/12] i2c/i2c-pxa: Remove conditional compilation of clk code Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:52   ` Wolfram Sang
2012-04-24 11:52     ` Wolfram Sang
2012-04-24 11:21 ` [PATCH V3 04/12] usb/marvell: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 05/12] usb/musb: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 06/12] ata/pata_arasan: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 07/12] ata/sata_mv: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 12:00   ` Andrew Lunn [this message]
2012-04-24 12:00     ` Andrew Lunn
2012-04-24 12:51     ` Lothar Waßmann
2012-04-24 12:51       ` Lothar Waßmann
2012-04-24 13:42     ` viresh kumar
2012-04-24 13:42       ` viresh kumar
2012-04-24 14:29       ` Andrew Lunn
2012-04-24 14:29         ` Andrew Lunn
2012-04-24 17:02         ` viresh kumar
2012-04-24 17:02           ` viresh kumar
2012-04-25  5:42           ` Andrew Lunn
2012-04-25  5:42             ` Andrew Lunn
2012-04-24 17:05         ` viresh kumar
2012-04-24 17:05           ` viresh kumar
2012-04-24 20:18       ` Russell King - ARM Linux
2012-04-24 20:18         ` Russell King - ARM Linux
2012-04-25  3:02         ` viresh kumar
2012-04-25  3:02           ` viresh kumar
2012-04-25  5:28           ` Andrew Lunn
2012-04-25  5:28             ` Andrew Lunn
2012-04-25  6:43             ` Lothar Waßmann
2012-04-25  6:43               ` Lothar Waßmann
2012-04-25  7:14               ` Andrew Lunn
2012-04-25  7:14                 ` Andrew Lunn
2012-04-25  8:35                 ` Lothar Waßmann
2012-04-25  8:35                   ` Lothar Waßmann
2012-04-25  9:31                   ` Andrew Lunn
2012-04-25  9:31                     ` Andrew Lunn
2012-04-25  9:31                     ` Andrew Lunn
2012-04-25 10:37                     ` Russell King - ARM Linux
2012-04-25 10:37                       ` Russell King - ARM Linux
2012-04-25 11:24         ` Andrew Lunn
2012-04-25 11:24           ` Andrew Lunn
2012-04-24 11:21 ` [PATCH V3 08/12] net/c_can: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 09/12] net/stmmac: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 10/12] gadget/m66592: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 11/12] gadget/r8a66597: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar
2012-04-24 11:21 ` [PATCH V3 12/12] usb/host/r8a66597: " Viresh Kumar
2012-04-24 11:21   ` Viresh Kumar

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=20120424120019.GE24089@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=akpm@linux-foundation.org \
    --cc=jgarzik@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mturquette@linaro.org \
    --cc=spear-devel@list.st.com \
    --cc=sshtylyov@mvista.com \
    --cc=viresh.kumar@st.com \
    --cc=viresh.linux@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.