All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-03  2:36 ` Nicholas Krause
  0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Krause @ 2014-07-03  2:36 UTC (permalink / raw)
  To: viresh.linux
  Cc: shiraz.linux.kernel, linux, spear-devel, linux-arm-kernel, linux-kernel

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(-)

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,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-03  2:36 ` Nicholas Krause
  0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Krause @ 2014-07-03  2:36 UTC (permalink / raw)
  To: linux-arm-kernel

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(-)

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,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mach-spear: fixed spear1340.c file
  2014-07-03  2:36 ` Nicholas Krause
@ 2014-07-03 18:04   ` Paul Bolle
  -1 siblings, 0 replies; 12+ messages in thread
From: Paul Bolle @ 2014-07-03 18:04 UTC (permalink / raw)
  To: Nicholas Krause
  Cc: viresh.linux, shiraz.linux.kernel, linux, spear-devel,
	linux-arm-kernel, linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-03 18:04   ` Paul Bolle
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Bolle @ 2014-07-03 18:04 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mach-spear: fixed spear1340.c file
  2014-07-03 18:04   ` Paul Bolle
@ 2014-07-03 18:08     ` Nick Krause
  -1 siblings, 0 replies; 12+ messages in thread
From: Nick Krause @ 2014-07-03 18:08 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Viresh Kumar, Shiraz Hashim, Russell King - ARM Linux,
	spear-devel, linux-arm-kernel, linux-kernel

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-03 18:08     ` Nick Krause
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Krause @ 2014-07-03 18:08 UTC (permalink / raw)
  To: linux-arm-kernel

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mach-spear: fixed spear1340.c file
  2014-07-03 18:08     ` Nick Krause
@ 2014-07-03 18:36       ` Paul Bolle
  -1 siblings, 0 replies; 12+ messages in thread
From: Paul Bolle @ 2014-07-03 18:36 UTC (permalink / raw)
  To: Nick Krause
  Cc: Viresh Kumar, Shiraz Hashim, Russell King - ARM Linux,
	spear-devel, linux-arm-kernel, linux-kernel

[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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-03 18:36       ` Paul Bolle
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Bolle @ 2014-07-03 18:36 UTC (permalink / raw)
  To: linux-arm-kernel

[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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mach-spear: fixed spear1340.c file
  2014-07-03 18:36       ` Paul Bolle
@ 2014-07-03 20:37         ` Nick Krause
  -1 siblings, 0 replies; 12+ messages in thread
From: Nick Krause @ 2014-07-03 20:37 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Viresh Kumar, Shiraz Hashim, Russell King - ARM Linux,
	spear-devel, linux-arm-kernel, linux-kernel

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-03 20:37         ` Nick Krause
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Krause @ 2014-07-03 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mach-spear: fixed spear1340.c file
  2014-07-03 20:37         ` Nick Krause
@ 2014-07-04  4:40           ` Viresh Kumar
  -1 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2014-07-04  4:40 UTC (permalink / raw)
  To: Nick Krause
  Cc: Paul Bolle, Shiraz Hashim, Russell King - ARM Linux, spear-devel,
	linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mach-spear: fixed spear1340.c file
@ 2014-07-04  4:40           ` Viresh Kumar
  0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2014-07-04  4:40 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-07-04  4:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-03  2:36 [PATCH 2/2] mach-spear: fixed spear1340.c file Nicholas Krause
2014-07-03  2:36 ` Nicholas Krause
2014-07-03 18:04 ` Paul Bolle
2014-07-03 18:04   ` Paul Bolle
2014-07-03 18:08   ` Nick Krause
2014-07-03 18:08     ` Nick Krause
2014-07-03 18:36     ` Paul Bolle
2014-07-03 18:36       ` Paul Bolle
2014-07-03 20:37       ` Nick Krause
2014-07-03 20:37         ` Nick Krause
2014-07-04  4:40         ` Viresh Kumar
2014-07-04  4:40           ` Viresh Kumar

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.