| Message ID | 1404354994-7478-1-git-send-email-xerofoify@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2014-07-02 at 22:36 -0400, Nicholas Krause wrote: > This is the fixed file after moving sata support to new file in > spear1340_sata.c > > Signed-off-by: Nicholas Krause <xerofoify@gmail.com> > --- > arch/arm/mach-spear/spear1340.c | 111 ---------------------------------------- > 1 file changed, 111 deletions(-) This patch, together with patch 1/2, basically moves a chunk of code into a separate file, didn't it? If so, why did you split that move in two patches? And how does all this work without any changes to a Makefile? > diff --git a/arch/arm/mach-spear/spear1340.c b/arch/arm/mach-spear/spear1340.c > index 7b6bff7..f9d8ef3 100644 > --- a/arch/arm/mach-spear/spear1340.c > +++ b/arch/arm/mach-spear/spear1340.c > @@ -21,117 +21,6 @@ > #include "generic.h" > #include <mach/spear.h> > > -/* FIXME: Move SATA PHY code into a standalone driver */ (I have no idea what this FIXME is about, but I do wonder whether that new file by itself is the standalone driver this FIXME is about. The spear developers will surely know.) Paul Bolle
Yes it is and I did it in two patches in order to be more readable. Furthermore I don't known Kconfig well enough to do the Makefile for the file I created. Cheers Nick On Thu, Jul 3, 2014 at 2:04 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Wed, 2014-07-02 at 22:36 -0400, Nicholas Krause wrote: >> This is the fixed file after moving sata support to new file in >> spear1340_sata.c >> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com> >> --- >> arch/arm/mach-spear/spear1340.c | 111 ---------------------------------------- >> 1 file changed, 111 deletions(-) > > This patch, together with patch 1/2, basically moves a chunk of code > into a separate file, didn't it? If so, why did you split that move in > two patches? > > And how does all this work without any changes to a Makefile? > >> diff --git a/arch/arm/mach-spear/spear1340.c b/arch/arm/mach-spear/spear1340.c >> index 7b6bff7..f9d8ef3 100644 >> --- a/arch/arm/mach-spear/spear1340.c >> +++ b/arch/arm/mach-spear/spear1340.c >> @@ -21,117 +21,6 @@ >> #include "generic.h" >> #include <mach/spear.h> >> >> -/* FIXME: Move SATA PHY code into a standalone driver */ > > (I have no idea what this FIXME is about, but I do wonder whether that > new file by itself is the standalone driver this FIXME is about. The > spear developers will surely know.) > > > Paul Bolle >
[I fixed the top posting.] On Thu, 2014-07-03 at 14:08 -0400, Nick Krause wrote: > On Thu, Jul 3, 2014 at 2:04 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > > This patch, together with patch 1/2, basically moves a chunk of code > > into a separate file, didn't it? If so, why did you split that move in > > two patches? > > Yes it is and I did it in two patches in order to be more readable. It makes it harder to understand the change (I had to _guess_ it was a move). Moreover, depending on the order that these two patches would be merged, we could end up with a chunk of code being either included twice or not included at all, in some range of commits. Neither would be good. > > And how does all this work without any changes to a Makefile? > > Furthermore I don't known Kconfig well enough to do the Makefile > for the file I created. Then I think you should, well, study the kernel build system before submitting a change like this. And you can also ask a question or two to get things going. (Not sure what the relevant list would be.) Paul Bolle
Very well then I will read the documentation on Kconfig in order to understand that and fix up my patches for that. On the other hand I will send a email before the patches to tell in what order to apply them. Cheers Nick On Thu, Jul 3, 2014 at 2:36 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > [I fixed the top posting.] > > On Thu, 2014-07-03 at 14:08 -0400, Nick Krause wrote: >> On Thu, Jul 3, 2014 at 2:04 PM, Paul Bolle <pebolle@tiscali.nl> wrote: >> > This patch, together with patch 1/2, basically moves a chunk of code >> > into a separate file, didn't it? If so, why did you split that move in >> > two patches? >> >> Yes it is and I did it in two patches in order to be more readable. > > It makes it harder to understand the change (I had to _guess_ it was a > move). Moreover, depending on the order that these two patches would be > merged, we could end up with a chunk of code being either included twice > or not included at all, in some range of commits. Neither would be good. > >> > And how does all this work without any changes to a Makefile? >> >> Furthermore I don't known Kconfig well enough to do the Makefile >> for the file I created. > > Then I think you should, well, study the kernel build system before > submitting a change like this. And you can also ask a question or two to > get things going. (Not sure what the relevant list would be.) > > > Paul Bolle >
Hi Nick, On Fri, Jul 4, 2014 at 2:07 AM, Nick Krause <xerofoify@gmail.com> wrote: > Very well then I will read the documentation on Kconfig in order to > understand that and fix up my patches for that. > On the other hand I will send a email before the patches to tell in > what order to apply them. As already asked by Paul, please don't top-post: http://en.wikipedia.org/wiki/Posting_style#Top-posting Firstly, in case we are going to get these patches in these have to be merged together, otherwise inbetween these two we will have duplicate functions for the same thing.. Kernel should work/build properly between commits, so that git bisect works.. Now about this: > -/* FIXME: Move SATA PHY code into a standalone driver */ I don't think it was just about creating another file for this. Otherwise how hard is it for the author then? It was probably more about updating the interface of the sata driver to accept stub drivers, so that we don't have to keep stuff here. So, this patch wouldn't fix the FIXME and so better drop this patchset completely. Thanks for trying though. --- Viresh
diff --git a/arch/arm/mach-spear/spear1340.c b/arch/arm/mach-spear/spear1340.c index 7b6bff7..f9d8ef3 100644 --- a/arch/arm/mach-spear/spear1340.c +++ b/arch/arm/mach-spear/spear1340.c @@ -21,117 +21,6 @@ #include "generic.h" #include <mach/spear.h> -/* FIXME: Move SATA PHY code into a standalone driver */ - -/* Base addresses */ -#define SPEAR1340_SATA_BASE UL(0xB1000000) - -/* Power Management Registers */ -#define SPEAR1340_PCM_CFG (VA_MISC_BASE + 0x100) -#define SPEAR1340_PCM_WKUP_CFG (VA_MISC_BASE + 0x104) -#define SPEAR1340_SWITCH_CTR (VA_MISC_BASE + 0x108) - -#define SPEAR1340_PERIP1_SW_RST (VA_MISC_BASE + 0x318) -#define SPEAR1340_PERIP2_SW_RST (VA_MISC_BASE + 0x31C) -#define SPEAR1340_PERIP3_SW_RST (VA_MISC_BASE + 0x320) - -/* PCIE - SATA configuration registers */ -#define SPEAR1340_PCIE_SATA_CFG (VA_MISC_BASE + 0x424) - /* PCIE CFG MASks */ - #define SPEAR1340_PCIE_CFG_DEVICE_PRESENT (1 << 11) - #define SPEAR1340_PCIE_CFG_POWERUP_RESET (1 << 10) - #define SPEAR1340_PCIE_CFG_CORE_CLK_EN (1 << 9) - #define SPEAR1340_PCIE_CFG_AUX_CLK_EN (1 << 8) - #define SPEAR1340_SATA_CFG_TX_CLK_EN (1 << 4) - #define SPEAR1340_SATA_CFG_RX_CLK_EN (1 << 3) - #define SPEAR1340_SATA_CFG_POWERUP_RESET (1 << 2) - #define SPEAR1340_SATA_CFG_PM_CLK_EN (1 << 1) - #define SPEAR1340_PCIE_SATA_SEL_PCIE (0) - #define SPEAR1340_PCIE_SATA_SEL_SATA (1) - #define SPEAR1340_SATA_PCIE_CFG_MASK 0xF1F - #define SPEAR1340_PCIE_CFG_VAL (SPEAR1340_PCIE_SATA_SEL_PCIE | \ - SPEAR1340_PCIE_CFG_AUX_CLK_EN | \ - SPEAR1340_PCIE_CFG_CORE_CLK_EN | \ - SPEAR1340_PCIE_CFG_POWERUP_RESET | \ - SPEAR1340_PCIE_CFG_DEVICE_PRESENT) - #define SPEAR1340_SATA_CFG_VAL (SPEAR1340_PCIE_SATA_SEL_SATA | \ - SPEAR1340_SATA_CFG_PM_CLK_EN | \ - SPEAR1340_SATA_CFG_POWERUP_RESET | \ - SPEAR1340_SATA_CFG_RX_CLK_EN | \ - SPEAR1340_SATA_CFG_TX_CLK_EN) - -#define SPEAR1340_PCIE_MIPHY_CFG (VA_MISC_BASE + 0x428) - #define SPEAR1340_MIPHY_OSC_BYPASS_EXT (1 << 31) - #define SPEAR1340_MIPHY_CLK_REF_DIV2 (1 << 27) - #define SPEAR1340_MIPHY_CLK_REF_DIV4 (2 << 27) - #define SPEAR1340_MIPHY_CLK_REF_DIV8 (3 << 27) - #define SPEAR1340_MIPHY_PLL_RATIO_TOP(x) (x << 0) - #define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \ - (SPEAR1340_MIPHY_OSC_BYPASS_EXT | \ - SPEAR1340_MIPHY_CLK_REF_DIV2 | \ - SPEAR1340_MIPHY_PLL_RATIO_TOP(60)) - #define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \ - (SPEAR1340_MIPHY_PLL_RATIO_TOP(120)) - #define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \ - (SPEAR1340_MIPHY_OSC_BYPASS_EXT | \ - SPEAR1340_MIPHY_PLL_RATIO_TOP(25)) - -/* SATA device registration */ -static int sata_miphy_init(struct device *dev, void __iomem *addr) -{ - writel(SPEAR1340_SATA_CFG_VAL, SPEAR1340_PCIE_SATA_CFG); - writel(SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK, - SPEAR1340_PCIE_MIPHY_CFG); - /* Switch on sata power domain */ - writel((readl(SPEAR1340_PCM_CFG) | (0x800)), SPEAR1340_PCM_CFG); - msleep(20); - /* Disable PCIE SATA Controller reset */ - writel((readl(SPEAR1340_PERIP1_SW_RST) & (~0x1000)), - SPEAR1340_PERIP1_SW_RST); - msleep(20); - - return 0; -} - -void sata_miphy_exit(struct device *dev) -{ - writel(0, SPEAR1340_PCIE_SATA_CFG); - writel(0, SPEAR1340_PCIE_MIPHY_CFG); - - /* Enable PCIE SATA Controller reset */ - writel((readl(SPEAR1340_PERIP1_SW_RST) | (0x1000)), - SPEAR1340_PERIP1_SW_RST); - msleep(20); - /* Switch off sata power domain */ - writel((readl(SPEAR1340_PCM_CFG) & (~0x800)), SPEAR1340_PCM_CFG); - msleep(20); -} - -int sata_suspend(struct device *dev) -{ - if (dev->power.power_state.event == PM_EVENT_FREEZE) - return 0; - - sata_miphy_exit(dev); - - return 0; -} - -int sata_resume(struct device *dev) -{ - if (dev->power.power_state.event == PM_EVENT_THAW) - return 0; - - return sata_miphy_init(dev, NULL); -} - -static struct ahci_platform_data sata_pdata = { - .init = sata_miphy_init, - .exit = sata_miphy_exit, - .suspend = sata_suspend, - .resume = sata_resume, -}; - /* Add SPEAr1340 auxdata to pass platform data */ static struct of_dev_auxdata spear1340_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA("snps,spear-ahci", SPEAR1340_SATA_BASE, NULL,
This is the fixed file after moving sata support to new file in spear1340_sata.c Signed-off-by: Nicholas Krause <xerofoify@gmail.com> --- arch/arm/mach-spear/spear1340.c | 111 ---------------------------------------- 1 file changed, 111 deletions(-)