| Message ID | abbcf661dc58948794839eb0fba1d5a0950ec848.1335266056.git.viresh.kumar@st.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
> -#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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Andrew Lunn writes: > > -#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. > The clock API should accept NULL pointers as valid clocks and treat them as NOP. Thus drivers wouldn't have to worry about whether they actually got a clock to manage or if their clocks are just dummies. Lothar Waßmann
On 4/24/12, Andrew Lunn <andrew@lunn.ch> wrote: > 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. Hmm. What this code does now is: If HAVE_CLK is selected, then there must be a clock for the device. Otherwise it will always pass. You want not to return error if a platform does have HAVE_CLK, but doesn't have a clock for sata? That would be simple to fix, but want to confirm if this is actually required. @Russell: Can we have your view also? -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 24, 2012 at 07:12:10PM +0530, viresh kumar wrote: > On 4/24/12, Andrew Lunn <andrew@lunn.ch> wrote: > > 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. > > Hmm. What this code does now is: > If HAVE_CLK is selected, then there must be a clock for the device. Otherwise > it will always pass. > -#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); There are three use cases.... 1) The platform does not implement HAVE_CLK. So we are using your NOP operations. clk_get() returns NULL. IS_ERR(NULL) is false. So it keeps going, calls clk_enable() which is also NOP and the driver is happy. 2) The platform does have HAVE_CLK. So we are using driver/clk/ code. There is no clk defined for the device, since there is no controllable clk. Its using the PCI clock, or some hard wired internal SoC clock. clk_get() returns ERR_PTR(-ENOENT) IS_ERR(hpriv->clk) is true, you output a dev_err() and the device probe fails. This is wrong. Not having the clk is not fatal. The old code would carefully not call clk_enable(), since it has an error pointer, not have a valid clk, and would also not call clk_disable during removal. 3) The platform does have HAVE_CLK. So we are using driver/clk/ code. There is however a gateable clock driving this lump of silicon. clk_get() returns a valid pointer to a clk. IS_ERR(hpriv->clk) is false, so it keeps going. clk_enable() is called and the driver is happy. Well, actually, his brings up a new issues static int __clk_enable(struct clk *clk) { int ret = 0; if (!clk) return 0; if (WARN_ON(clk->prepare_count == 0)) return -ESHUTDOWN; this should actually be clk_prepare_enable(). Did you see my patches adding generic clk framework support for Orion. I fixed this as part of that patch set. Anyway.... You asked: > You want not to return error if a platform does have HAVE_CLK, but > doesn't have a clock for sata? That would be simple to fix, but want > to confirm if this is actually required. Correct. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/24/12, Andrew Lunn <andrew@lunn.ch> wrote: > static int __clk_enable(struct clk *clk) > { > int ret = 0; > > if (!clk) > return 0; > > if (WARN_ON(clk->prepare_count == 0)) > return -ESHUTDOWN; > > this should actually be clk_prepare_enable(). Did you see my > patches adding generic clk framework support for Orion. I fixed > this as part of that patch set. Sorry for missing it earlier. Ya i know it must be clk_prepare_enable(), but it wasn't motive of my patchset. Sorry, i haven't gone through your patches for Orion. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 24, 2012 at 07:12:10PM +0530, viresh kumar wrote: > On 4/24/12, Andrew Lunn <andrew@lunn.ch> wrote: > > 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. > > Hmm. What this code does now is: > If HAVE_CLK is selected, then there must be a clock for the device. Otherwise > it will always pass. > > You want not to return error if a platform does have HAVE_CLK, but doesn't > have a clock for sata? That would be simple to fix, but want to confirm if this > is actually required. > > @Russell: Can we have your view also? Look, it's very very simple. As far as drivers are concerned: clk_get() returns an opaque cookie which _happens_ to be called 'struct clk'. Drivers _must_ _not_ dereference or interpret this cookie in any way, apart from the singular case where they use IS_ERR() to determine if clk_get() failed, and PTR_ERR() to translate that into an error value. As far as drivers are concerned _everything_ _else_ is a valid cookie and must never be treated specially. That much I hope is (and has been) totally crystal clear for some time. Now, for drivers which use the clk API, and are used on platforms which have the clk API and those which do not have the CLK API. Those which do have the clk API define HAVE_CLK. We know how to deal with those, and that's through having a correct and valid clk API implementation. For those which don't, as I've already suggested, we need clk_get() to return a non-error value. I really don't care what value it returns, because as far as drivers using the clk API are concerned, they are not allowed to interpret the value in _any_ _other_ _way_ other than whether it is an error or not. So NULL is a good value for this. It's a non-error cookie value, but (void *)1 is also good too. Now, the question comes: do we want to provide a dummy API? Yes. How do we want to enable the provision of the dummy API? Through !HAVE_CLK? I think that's a sane move, and any driver which _really_ _does_ have a hard dependency on the clk API (eg, amba-clcd needing the clk API to control the LCD pixel clock rate) should depend on this symbol. As for drivers printing out crap if they don't have the clk API configured, wtf? What does it matter? If the clk API is not configured, it means the platform has no control over the clocking, and the clocking is fixed. So why tell the user of each driver which could have clk API support that same fact over and over again during the kernel boot process? What do you expect the user to do about it? Scream at the manufacturer that they didn't implement a feature found on embedded devices on their swankey platform? Maybe its not appropriate there? Finally, if a platform has clk API support enabled, and a driver requests a clock, and clk_get() returns an error, it means the clock was not found. That's a fatal error for the driver, because it means that something is wrong in the lookup tables - moreover, it means that _potentially_ someone screwed up the clk matching and this device has a clock which needs some control, but wasn't found. I don't think ignoring that kind of error, even by printing a warning, is a particularly good approach - it seems to me it makes things fragile. What if this missing clock causes the bus to your device to ultimately hang? -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 25, 2012 at 1:48 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > Look, it's very very simple. Thanks for explaining. > Finally, if a platform has clk API support enabled, and a driver requests > a clock, and clk_get() returns an error, it means the clock was not found. > That's a fatal error for the driver, because it means that something is > wrong in the lookup tables - moreover, it means that _potentially_ someone > screwed up the clk matching and this device has a clock which needs some > control, but wasn't found. I don't think ignoring that kind of error, > even by printing a warning, is a particularly good approach - it seems > to me it makes things fragile. What if this missing clock causes the > bus to your device to ultimately hang? This is what i was thinking too and thats why floated this version of patch. But as Andrew said, clk API support is enabled for them, but still they don't have a clk for sata. To get this working, there are two solutions: - Create dummy clk for sata for that platform, so clk_get doesn't fail. - Check for error before every call to clk APIs after clk_get(). Andrew favored the second one. Which means, even on platforms with clk API defined and clk enable required, if there are some issues with lookup table, and clk_get() fails, system may hang when registers are accessed. For this i favored first one. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> This is what i was thinking too and thats why floated this version of patch. > But as Andrew said, clk API support is enabled for them, but still they > don't have a clk for sata. To get this working, there are two solutions: > - Create dummy clk for sata for that platform, so clk_get doesn't fail. > - Check for error before every call to clk APIs after clk_get(). > > Andrew favored the second one. Which means, even on platforms > with clk API defined and clk enable required, if there are some issues > with lookup table, and clk_get() fails, system may hang when registers > are accessed. For this i favored first one. I'm not too sure how you are going to achieve 1) config SATA_MV tristate "Marvell SATA support" help This option enables support for the Marvell Serial ATA family. Currently supports 88SX[56]0[48][01] PCI(-X) chips, as well as the newer [67]042 PCI-X/PCIe and SOC devices. So this driver can be used with anything which has a PCI(-X) or PCIe bus, or Orion SoC and a few PowerPC SoCs. The SoCs are not too bad, we know which ones they are and we can add dummy entries to their device tree. However, how do you want to handle the PCI devices? Create the dummy entry somewhere in the middle of the PCI core? Thought not. A comment to: > - Check for error before every call to clk APIs after clk_get(). There are only two calls. clk_prepare_enable() in the probe and clk_disable_unprepare() in the remove function. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Andrew Lunn writes: > > This is what i was thinking too and thats why floated this version of patch. > > But as Andrew said, clk API support is enabled for them, but still they > > don't have a clk for sata. To get this working, there are two solutions: > > - Create dummy clk for sata for that platform, so clk_get doesn't fail. > > - Check for error before every call to clk APIs after clk_get(). > > > > Andrew favored the second one. Which means, even on platforms > > with clk API defined and clk enable required, if there are some issues > > with lookup table, and clk_get() fails, system may hang when registers > > are accessed. For this i favored first one. > > I'm not too sure how you are going to achieve 1) > > config SATA_MV > tristate "Marvell SATA support" > help > This option enables support for the Marvell Serial ATA family. > Currently supports 88SX[56]0[48][01] PCI(-X) chips, > as well as the newer [67]042 PCI-X/PCIe and SOC devices. > > So this driver can be used with anything which has a PCI(-X) or PCIe > bus, or Orion SoC and a few PowerPC SoCs. > > The SoCs are not too bad, we know which ones they are and we can add > dummy entries to their device tree. However, how do you want to handle > the PCI devices? Create the dummy entry somewhere in the middle of the > PCI core? > > Thought not. > AFAICT the PCI archs do not have HAVE_CLK enabled. Thus they will have a dummy clk API in place that provides the driver with a non-error clk_get() return value. Lothar Waßmann
> AFAICT the PCI archs do not have HAVE_CLK enabled. Thus they will have > a dummy clk API in place that provides the driver with a non-error > clk_get() return value. Hi Lothar Please could you explain this a little bit more. What exactly is a PCI arch? Most of the Orion SoC have PCIe busses. Does this make them an PCI arch? Because they also have HAVE_CLK. There are more SoCs with PCI. Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Andrew Lunn writes: > > AFAICT the PCI archs do not have HAVE_CLK enabled. Thus they will have > > a dummy clk API in place that provides the driver with a non-error > > clk_get() return value. > > Hi Lothar > > Please could you explain this a little bit more. What exactly is a PCI > arch? > > Most of the Orion SoC have PCIe busses. Does this make them an PCI > arch? Because they also have HAVE_CLK. There are more SoCs with PCI. > If an arch has HAVE_CLK enabled it must provide valid clocks (be it dummy clocks) for all devices it supports. Lothar Waßmann
> If an arch has HAVE_CLK enabled it must provide valid clocks (be it > dummy clocks) for all devices it supports. So, lets take the theoretical exaple of a unicore32 PUV3 config ARCH_PUV3 def_bool y select CPU_UCV2 select GENERIC_CLOCKEVENTS select HAVE_CLK select ARCH_REQUIRE_GPIOLIB select ARCH_HAS_CPUFREQ Seems like this somewhat unknown, to me at least, architecture, also supports PCI. So i plug in an HP Adaptec AIC8120 SATA host bus adapter into a spare PCI slot. This uses the Marvell 88SX6041, which the SATA_MV driver supports. Should i expect that the unicore32 PUV3 has created a dummy clk for this case? Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 25, 2012 at 11:31:09AM +0200, Andrew Lunn wrote: > > If an arch has HAVE_CLK enabled it must provide valid clocks (be it > > dummy clocks) for all devices it supports. > > So, lets take the theoretical exaple of a unicore32 PUV3 > > config ARCH_PUV3 > def_bool y > select CPU_UCV2 > select GENERIC_CLOCKEVENTS > select HAVE_CLK > select ARCH_REQUIRE_GPIOLIB > select ARCH_HAS_CPUFREQ > > Seems like this somewhat unknown, to me at least, architecture, also > supports PCI. So i plug in an HP Adaptec AIC8120 SATA host bus adapter > into a spare PCI slot. This uses the Marvell 88SX6041, which the > SATA_MV driver supports. > > Should i expect that the unicore32 PUV3 has created a dummy clk for > this case? Are you going to bother answering the detailed email I sent describing my POV on this case? -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 24, 2012 at 09:18:02PM +0100, Russell King - ARM Linux wrote: > On Tue, Apr 24, 2012 at 07:12:10PM +0530, viresh kumar wrote: > > On 4/24/12, Andrew Lunn <andrew@lunn.ch> wrote: > > > 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. > > > > Hmm. What this code does now is: > > If HAVE_CLK is selected, then there must be a clock for the device. Otherwise > > it will always pass. > > > > You want not to return error if a platform does have HAVE_CLK, but doesn't > > have a clock for sata? That would be simple to fix, but want to confirm if this > > is actually required. > > > > @Russell: Can we have your view also? > > Look, it's very very simple. > > As far as drivers are concerned: > > clk_get() returns an opaque cookie which _happens_ to be called 'struct clk'. > Drivers _must_ _not_ dereference or interpret this cookie in any way, apart > from the singular case where they use IS_ERR() to determine if clk_get() > failed, and PTR_ERR() to translate that into an error value. As far as > drivers are concerned _everything_ _else_ is a valid cookie and must > never be treated specially. > > That much I hope is (and has been) totally crystal clear for some time. > > Now, for drivers which use the clk API, and are used on platforms which > have the clk API and those which do not have the CLK API. Those which > do have the clk API define HAVE_CLK. We know how to deal with those, > and that's through having a correct and valid clk API implementation. > > For those which don't, as I've already suggested, we need clk_get() to > return a non-error value. I really don't care what value it returns, > because as far as drivers using the clk API are concerned, they are not > allowed to interpret the value in _any_ _other_ _way_ other than whether > it is an error or not. So NULL is a good value for this. It's a > non-error cookie value, but (void *)1 is also good too. > > Now, the question comes: do we want to provide a dummy API? Yes. How > do we want to enable the provision of the dummy API? Through !HAVE_CLK? > I think that's a sane move, and any driver which _really_ _does_ have a > hard dependency on the clk API (eg, amba-clcd needing the clk API to > control the LCD pixel clock rate) should depend on this symbol. > > As for drivers printing out crap if they don't have the clk API configured, > wtf? What does it matter? If the clk API is not configured, it means > the platform has no control over the clocking, and the clocking is fixed. > So why tell the user of each driver which could have clk API support that > same fact over and over again during the kernel boot process? What do you > expect the user to do about it? Scream at the manufacturer that they > didn't implement a feature found on embedded devices on their swankey > platform? Maybe its not appropriate there? > Hi Russell No problems with what is above. The bit in contention is this > Finally, if a platform has clk API support enabled, and a driver requests > a clock, and clk_get() returns an error, it means the clock was not found. > That's a fatal error for the driver, because it means that something is > wrong in the lookup tables - moreover, it means that _potentially_ someone > screwed up the clk matching and this device has a clock which needs some > control, but wasn't found. I don't think ignoring that kind of error, > even by printing a warning, is a particularly good approach - it seems > to me it makes things fragile. What if this missing clock causes the > bus to your device to ultimately hang? You are correct about lockup. I made a typo, match failed, lateinit turned the clock off, and the device hung on the next access. Is that fragile? It should only happen to somebody porting to a new SoC playing with clks. Anyway, what you want, is that the MV_SATA driver knows if there should be a clock and only then call clk_get(). How do we reliably teach the MV_SATA driver this, so we don't cause an regressions? If this platform_device is for a PCI bus device, there probably won't be a clock. If this is a SoC platform_device is for a Dove, Orion5x, mv78xx0, there won't be a clock. If its a kirkwood SoC platform device, there should be a clock. If its a PowerPC platform_device, i've no idea. Have i missed something? It seems to come down to, a bit of fragile handling of clks, or possibly regressing some PowerPC machines. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 7336d4a..2b37b93 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -551,9 +551,7 @@ struct mv_host_priv { u32 irq_mask_offset; u32 unmask_all_irqs; -#if defined(CONFIG_HAVE_CLK) struct clk *clk; -#endif /* * These consistent DMA memory pools give us guaranteed * alignment for hardware-accessed data structures, @@ -4063,13 +4061,13 @@ static int mv_platform_probe(struct platform_device *pdev) resource_size(res)); hpriv->base -= SATAHC0_REG_BASE; -#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); /* * (Re-)program MBUS remapping windows if we are asked to. @@ -4096,12 +4094,8 @@ static int mv_platform_probe(struct platform_device *pdev) return 0; err: -#if defined(CONFIG_HAVE_CLK) - if (!IS_ERR(hpriv->clk)) { - clk_disable(hpriv->clk); - clk_put(hpriv->clk); - } -#endif + clk_disable(hpriv->clk); + clk_put(hpriv->clk); return rc; } @@ -4117,17 +4111,11 @@ err: static int __devexit mv_platform_remove(struct platform_device *pdev) { struct ata_host *host = platform_get_drvdata(pdev); -#if defined(CONFIG_HAVE_CLK) struct mv_host_priv *hpriv = host->private_data; -#endif ata_host_detach(host); -#if defined(CONFIG_HAVE_CLK) - if (!IS_ERR(hpriv->clk)) { - clk_disable(hpriv->clk); - clk_put(hpriv->clk); - } -#endif + clk_disable(hpriv->clk); + clk_put(hpriv->clk); return 0; }
With addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros. Signed-off-by: Viresh Kumar <viresh.kumar@st.com> Cc: Jeff Garzik <jgarzik@redhat.com> Cc: Andrew Lunn <andrew@lunn.ch> Cc: linux-ide@vger.kernel.org --- drivers/ata/sata_mv.c | 32 ++++++++++---------------------- 1 files changed, 10 insertions(+), 22 deletions(-)