[PATCH 1/3] ahci: st: Provide DT bindings for ST's SATA implementation

[PATCH 1/3] ahci: st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

Cc: devicetree at vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
new file mode 100644
index 0000000..1b69fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
@@ -0,0 +1,31 @@
+STMicroelectronics STi SATA controller
+
+This binding describes a SATA device.
+
+Required properties:
+ - compatible	   : Must be "st,ahci"
+ - reg		   : Physical base addresses and length of register sets
+ - interrupts	   : Interrupt associated with the SATA device
+ - interrupt-names :   Associated name must be; "hostc"
+ - resets	   : The power-down and soft-reset lines of SATA IP
+ - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
+ - clocks	   : The phandle for the clock
+ - clock-names	   :   Associated name must be; "ahci_clk"
+ - phys		   : The phandle for the PHY device
+ - phy-names	   :   Associated name must be; "ahci_phy"
+
+Example:
+
+	sata0: sata at fe380000 {
+		compatible      = "st,ahci";
+		reg             = <0xfe380000 0x1000>;
+		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+		interrupt-names = "hostc";
+		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+		phy-names       = "ahci_phy";
+		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+				  <&softreset STIH416_SATA0_SOFTRESET>;
+		reset-names     = "pwr-dwn", "sw-rst";
+		clocks	        = <&CLK_S_ICN_REG_0>;
+		clock-names     = "ahci_clk";
+	};
-- 
1.8.3.2

[PATCH 2/3] ARM: DT: STi: Add DT node for ST's SATA device

[Lee Jones]

Cc: devicetree at vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih416-b2020-revE.dts |  4 ++++
 arch/arm/boot/dts/stih416-b2020.dts      |  4 ++++
 arch/arm/boot/dts/stih416.dtsi           | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
index 693d0ec..7350a86 100644
--- a/arch/arm/boot/dts/stih416-b2020-revE.dts
+++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
@@ -37,5 +37,9 @@
 			st,pcie-tx-pol-inv;
 			st,sata-gen = <3>;
 		};
+
+		sata0: sata at fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
index fd9cbad..ebd784b 100644
--- a/arch/arm/boot/dts/stih416-b2020.dts
+++ b/arch/arm/boot/dts/stih416-b2020.dts
@@ -18,5 +18,9 @@
 			st,pcie_tx_pol_inv = <1>;
 			st,sata_gen = "gen3";
 		};
+
+		sata0: sata at fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 9fd8efb..d1c874f 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -153,5 +153,21 @@
 			#phy-cells 	= <2>;
 			st,syscfg  	= <&syscfg_rear>;
 		};
+
+		sata0: sata at fe380000 {
+			compatible      = "st,ahci";
+			reg             = <0xfe380000 0x1000>;
+			interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+			interrupt-names = "hostc";
+			phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+			phy-names       = "ahci_phy";
+			resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+					  <&softreset STIH416_SATA0_SOFTRESET>;
+			reset-names     = "pwr-dwn", "sw-rst";
+			clock-names     = "ahci_clk";
+			clocks	        = <&CLK_S_ICN_REG_0>;
+
+			status	        = "disabled";
+		};
 	};
 };
-- 
1.8.3.2

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

Cc: Tejun Heo <tj@kernel.org>
Cc: linux-ide at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/ata/Kconfig   |   9 ++
 drivers/ata/Makefile  |   1 +
 drivers/ata/ahci_st.c | 275 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 285 insertions(+)
 create mode 100644 drivers/ata/ahci_st.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 4e73772..a0798f3 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -97,6 +97,15 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config SATA_AHCI_ST
+	tristate "ST SATA support"
+	depends on SATA_AHCI_PLATFORM
+	select GENERIC_PHY
+	help
+	  This option enables support for ST SATA controller.
+
+	  If unsure, say N.
+
 config AHCI_IMX
 	tristate "Freescale i.MX AHCI SATA support"
 	depends on SATA_AHCI_PLATFORM && MFD_SYSCON
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 46518c6..3379a7d 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ATA)		+= libata.o
 obj-$(CONFIG_SATA_AHCI)		+= ahci.o libahci.o
 obj-$(CONFIG_SATA_ACARD_AHCI)	+= acard-ahci.o libahci.o
 obj-$(CONFIG_SATA_AHCI_PLATFORM) += ahci_platform.o libahci.o
+obj-$(CONFIG_SATA_AHCI_ST)      += ahci_st.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
new file mode 100644
index 0000000..23342bf
--- /dev/null
+++ b/drivers/ata/ahci_st.c
@@ -0,0 +1,275 @@
+/*
+ * Copyright (C) 2012 STMicroelectronics Limited
+ *
+ * Authors: Francesco Virlinzi <francesco.virlinzi@st.com>
+ *	    Alexandre Torgue <alexandre.torgue@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/ahci_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/reset.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+
+#define AHCI_OOBR		0xbc
+#define AHCI_OOBR_WE		BIT(31)
+#define AHCI_OOBR_CWMIN_SHIFT	24
+#define AHCI_OOBR_CWMAX_SHIFT	16
+#define AHCI_OOBR_CIMIN_SHIFT	8
+#define AHCI_OOBR_CIMAX_SHIFT	0
+
+struct ahci_st_drv_data {
+	struct platform_device *ahci;
+	struct phy *phy;
+	struct reset_control *pwr;
+	struct reset_control *sw_rst;
+	struct reset_control *pwr_rst;
+	void __iomem *mmio;
+	struct clk *clk;
+};
+
+static void ahci_st_configure_oob(void __iomem *mmio)
+{
+	unsigned long old_val, new_val;
+
+	new_val = (0x02 << AHCI_OOBR_CWMIN_SHIFT) |
+		  (0x04 << AHCI_OOBR_CWMAX_SHIFT) |
+		  (0x08 << AHCI_OOBR_CIMIN_SHIFT) |
+		  (0x0C << AHCI_OOBR_CIMAX_SHIFT);
+
+	old_val = readl(mmio + AHCI_OOBR);
+	writel(old_val | AHCI_OOBR_WE, mmio + AHCI_OOBR);
+	writel(new_val | AHCI_OOBR_WE, mmio + AHCI_OOBR);
+	writel(new_val, mmio + AHCI_OOBR);
+}
+
+static int ahci_st_probe_resets(struct platform_device *pdev,
+				struct ahci_st_drv_data *ahci_dev)
+{
+	int err;
+
+	ahci_dev->pwr = devm_reset_control_get(&pdev->dev, "pwr-dwn");
+	if (IS_ERR(ahci_dev->pwr)) {
+		dev_info(&pdev->dev, "power reset control not defined\n");
+	} else {
+		err = reset_control_deassert(ahci_dev->pwr);
+		if (err) {
+			dev_err(&pdev->dev, "unable to bring out of pwrdwn\n");
+			return err;
+		}
+	}
+
+	ahci_dev->sw_rst = devm_reset_control_get(&pdev->dev, "sw-rst");
+	if (IS_ERR(ahci_dev->sw_rst)) {
+		dev_info(&pdev->dev, "soft reset control not defined\n");
+	} else {
+		err = reset_control_deassert(ahci_dev->sw_rst);
+		if (err) {
+			dev_err(&pdev->dev, "unable to bring out of reset\n");
+			return err;
+		}
+	}
+
+	ahci_dev->pwr_rst = devm_reset_control_get(&pdev->dev, "pwr-rst");
+	if (IS_ERR(ahci_dev->pwr_rst))
+		dev_dbg(&pdev->dev, "power soft reset control not defined\n");
+	else {
+		err = reset_control_deassert(ahci_dev->pwr_rst);
+		if (err) {
+			dev_err(&pdev->dev, "unable to bring out of reset\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int ahci_st_init(struct device *ahci_dev, void __iomem *mmio)
+{
+	struct platform_device *pdev = to_platform_device(ahci_dev->parent);
+	struct ahci_st_drv_data *drv_data = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = clk_prepare_enable(drv_data->clk);
+	if (ret)
+		return ret;
+
+	ret = phy_init(drv_data->phy);
+	if (ret) {
+		clk_disable_unprepare(drv_data->clk);
+		return ret;
+	}
+
+	drv_data->mmio = mmio;
+	ahci_st_configure_oob(mmio);
+
+	return 0;
+}
+
+static void ahci_st_exit(struct device *ahci_dev)
+{
+	struct platform_device *pdev = to_platform_device(ahci_dev->parent);
+	struct ahci_st_drv_data *drv_data = platform_get_drvdata(pdev);
+	int ret;
+
+	if (drv_data->pwr) {
+		ret = reset_control_assert(drv_data->pwr);
+		if (ret)
+			dev_err(&pdev->dev, "unable to bring out of pwrdwn\n");
+	}
+
+	clk_disable_unprepare(drv_data->clk);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ahci_st_suspend(struct device *ahci_dev)
+{
+	struct platform_device *pdev = to_platform_device(ahci_dev->parent);
+	struct ahci_st_drv_data *drv_data = platform_get_drvdata(pdev);
+	int ret;
+
+	if (drv_data->pwr) {
+		ret = reset_control_assert(drv_data->pwr);
+		if (ret) {
+			dev_err(&pdev->dev, "unable to pwrdwn");
+			return ret;
+		}
+	}
+
+	clk_disable_unprepare(drv_data->clk);
+
+	return 0;
+}
+
+static int ahci_st_resume(struct device *ahci_dev)
+{
+	struct platform_device *pdev = to_platform_device(ahci_dev->parent);
+	struct ahci_st_drv_data *drv_data = platform_get_drvdata(pdev);
+	int ret;
+
+	clk_prepare_enable(drv_data->clk);
+
+	if (drv_data->pwr) {
+		ret = reset_control_deassert(drv_data->pwr);
+		if (ret) {
+			dev_err(&pdev->dev, "unable to bring out of pwrdwn\n");
+			return ret;
+		}
+	}
+
+	ahci_st_configure_oob(drv_data->mmio);
+
+	ret = phy_init(drv_data->phy);
+	if (ret)
+		return ret;
+
+	if (drv_data->sw_rst) {
+		ret = reset_control_deassert(drv_data->sw_rst);
+		if (ret) {
+			dev_err(&pdev->dev, "unable to bring out of sw-rst\n");
+			return ret;
+		}
+	}
+
+	if (drv_data->pwr_rst) {
+		ret = reset_control_deassert(drv_data->pwr_rst);
+		if (ret) {
+			dev_err(&pdev->dev, "unable to bring out of pwr-rst\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+#endif
+
+static struct ahci_platform_data ahci_st_platform_data = {
+	.init = ahci_st_init,
+	.exit = ahci_st_exit,
+#ifdef CONFIG_PM_SLEEP
+	.suspend = ahci_st_suspend,
+	.resume = ahci_st_resume,
+#endif
+};
+
+static int ahci_st_driver_probe(struct platform_device *pdev)
+{
+	int err, id;
+	struct ahci_st_drv_data *drv_data;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct platform_device_info ahci_info = {
+		.parent    = dev,
+		.name      = "ahci",
+		.id        = pdev->id,
+		.res       = pdev->resource,
+		.num_res   = pdev->num_resources,
+		.data      = &ahci_st_platform_data,
+		.size_data = sizeof(ahci_st_platform_data),
+	};
+
+	if (np) {
+		id = of_alias_get_id(dev->of_node, "sata");
+		if (id > 0)
+			ahci_info.id = id;
+
+		ahci_info.dma_mask = DMA_BIT_MASK(32);
+	}
+
+	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data)
+		return -ENOMEM;
+
+	err = ahci_st_probe_resets(pdev, drv_data);
+	if (err)
+		return err;
+
+	drv_data->clk = devm_clk_get(&pdev->dev, "ahci_clk");
+	if (IS_ERR(drv_data->clk)) {
+		dev_err(&pdev->dev, "ahci_clk clk not found\n");
+		return PTR_ERR(drv_data->clk);
+	}
+
+	drv_data->phy = devm_phy_get(&pdev->dev, "ahci_phy");
+	if (IS_ERR(drv_data->phy)) {
+		dev_err(&pdev->dev, "no PHY configured\n");
+		return PTR_ERR(drv_data->phy);
+	}
+
+	platform_set_drvdata(pdev, drv_data);
+
+	drv_data->ahci = platform_device_register_full(&ahci_info);
+
+	return PTR_ERR_OR_ZERO(drv_data->ahci);
+}
+
+static struct of_device_id st_ahci_match[] = {
+	{ .compatible = "st,ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_ahci_match);
+
+static struct platform_driver ahci_st_driver = {
+	.driver = {
+		.name = "ahci_st",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(st_ahci_match),
+	},
+	.probe = ahci_st_driver_probe,
+};
+module_platform_driver(ahci_st_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@st.com>");
+MODULE_AUTHOR("Francesco Virlinzi <francesco.virlinzi@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics Sata Ahci driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.2

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Mon, Feb 17, 2014 at 12:56:58PM +0000, Lee Jones wrote:
> Cc: Tejun Heo <tj@kernel.org>
> Cc: linux-ide at vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Please consider basing it on top of Hans' refactored ahci_platform.

 http://www.spinics.net/lists/linux-ide/msg47628.html

Thanks.

-- 
tejun

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: linux-ide at vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Please consider basing it on top of Hans' refactored ahci_platform.
> 
>  http://www.spinics.net/lists/linux-ide/msg47628.html

Okay, looks reasonable. I'm happy to do the extra work.

Can I do this as a subsequent patch though? The current patch fits
neatly onto our internal stable kernel (currently v3.10). The added
work will require back-porting Hans' patch-set, which has not yet
reached Mainline.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Hans de Goede]

Hi,

On 02/19/2014 12:36 AM, Tejun Heo wrote:
> On Mon, Feb 17, 2014 at 12:56:58PM +0000, Lee Jones wrote:
>> Cc: Tejun Heo <tj@kernel.org>
>> Cc: linux-ide at vger.kernel.org
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Please consider basing it on top of Hans' refactored ahci_platform.
> 
>  http://www.spinics.net/lists/linux-ide/msg47628.html

Which reminds I must test the version I've rebased on top of 3.14-rc1
and then send a v6 so that this can get merged.

Lee, I'll add you to the CC of the v6 I'm about to send.

Regards,

Hans

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> >> Cc: Tejun Heo <tj@kernel.org>
> >> Cc: linux-ide at vger.kernel.org
> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > Please consider basing it on top of Hans' refactored ahci_platform.
> > 
> >  http://www.spinics.net/lists/linux-ide/msg47628.html
> 
> Which reminds I must test the version I've rebased on top of 3.14-rc1
> and then send a v6 so that this can get merged.
> 
> Lee, I'll add you to the CC of the v6 I'm about to send.

Thanks Hans.

Once Tejun has accepted this driver I will go ahead and rebase on top
of your work and convert to the new API.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

Hello, Lee.

On Wed, Feb 19, 2014 at 08:30:25AM +0000, Lee Jones wrote:
> Can I do this as a subsequent patch though? The current patch fits
> neatly onto our internal stable kernel (currently v3.10). The added
> work will require back-porting Hans' patch-set, which has not yet
> reached Mainline.

Hmm.... I'll apply Hans' patchset pretty soon and we still have some
time in this devel cycle so I'd much prefer applying the final patch.

Thanks.

-- 
tejun

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > Can I do this as a subsequent patch though? The current patch fits
> > neatly onto our internal stable kernel (currently v3.10). The added
> > work will require back-porting Hans' patch-set, which has not yet
> > reached Mainline.
> 
> Hmm.... I'll apply Hans' patchset pretty soon and we still have some
> time in this devel cycle so I'd much prefer applying the final patch.

I think that's a pretty unfair ask. I could understand if the
patch(es) didn't apply, or were hindered by the new API change, but I
do not believe that is the case?

FWIW, I have tried to apply the patches Hans send me, but they don't
seem to apply cleanly.

Base is v3.14-rc1:

Applying: ahci-platform: Add support for devices with more then 1 clock
fatal: sha1 information is lacking or useless (drivers/ata/ahci_platform.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ahci-platform: Add support for devices with more then 1 clock
The copy of the patch that failed is found in:
   .git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Any ideas?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

Hello, Lee.

On Wed, Feb 19, 2014 at 03:01:21PM +0000, Lee Jones wrote:
> I think that's a pretty unfair ask. I could understand if the
> patch(es) didn't apply, or were hindered by the new API change, but I
> do not believe that is the case?

Let me make it very clear to you.  If you want your code in, you help
maintaining the larger code base.  I'm already extremely annoyed about
the general behavior of embedded developers where the general attitude
seems to be doing whatever to get *my* driver in the tree.  So, fuck
you a little bit and if you want to make it happen faster, help Hans'
patchset mature.  Unfair my ass.

Thanks.

-- 
tejun

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > I think that's a pretty unfair ask. I could understand if the
> > patch(es) didn't apply, or were hindered by the new API change, but I
> > do not believe that is the case?
> 
> Let me make it very clear to you.  If you want your code in, you help
> maintaining the larger code base.  I'm already extremely annoyed about
> the general behavior of embedded developers where the general attitude
> seems to be doing whatever to get *my* driver in the tree.  So, fuck
> you a little bit and if you want to make it happen faster, help Hans'
> patchset mature.  Unfair my ass.

Since when has maintaining core code been the responsibility of the
leaf driver developers? If you're aware that the core code is
sub-standard then it's you who should be fixing it.

I think it's great that forward thinking developers like Hans take on
challenges to improve subsystems which are lacking in one way or
another, but holding back other development while this process is
ongoing is fundamentally wrong. Especially in this case where you're
still actively reviewing/NACKing the core changes.

Changes to APIs should either support backward-capability or change all
effected drivers. I haven't been able to apply the patches yet, so I
can't tell which one of these holds true, but I believe it's the
former. In which case any new driver using the _current_ (not old)
API should fold neatly in. I've even offered to convert to the new API
once it's Mainlined. How can I say fairer than that?

Finally, do try and stay at least a little bit professional on the
MLs. That sort of disrespectful, rude behaviour may be how you guys do
it at Redhat, but most will think it's nothing more than childish.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Wed, Feb 19, 2014 at 03:23:36PM +0000, Lee Jones wrote:
> Since when has maintaining core code been the responsibility of the
> leaf driver developers? If you're aware that the core code is
> sub-standard then it's you who should be fixing it.

No, there isn't this clear divide between core and the leaf
developers.  People working on the "core" don't just grow off the
ground.  You can't just go around piling mess around claiming that
it's not your responsbility and then expect someone else to clean up
after you.  How can that be *possibly* sustainable?

> I think it's great that forward thinking developers like Hans take on
> challenges to improve subsystems which are lacking in one way or

Yeah, which happened only because I pushed back and Hans isn't even
paid to do it.  Doesn't that say something?  Nobody is actually
helping Hans' work.  Not at all, nada, zilch.  Just complain when
directed towards it.

> another, but holding back other development while this process is
> ongoing is fundamentally wrong. Especially in this case where you're
> still actively reviewing/NACKing the core changes.
> 
> Changes to APIs should either support backward-capability or change all
> effected drivers. I haven't been able to apply the patches yet, so I
> can't tell which one of these holds true, but I believe it's the
> former. In which case any new driver using the _current_ (not old)
> API should fold neatly in. I've even offered to convert to the new API
> once it's Mainlined. How can I say fairer than that?
> 
> Finally, do try and stay at least a little bit professional on the
> MLs. That sort of disrespectful, rude behaviour may be how you guys do
> it at Redhat, but most will think it's nothing more than childish.

If you want me to be respectful towards you, don't be a crybaby
screaming "it's not fair".  It apparently isn't clear to you that you
have to chip in for the whole thing to be maintainable in the long
term.  Unfair... lol, that was a good one, really.

-- 
tejun

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > Since when has maintaining core code been the responsibility of the
> > leaf driver developers? If you're aware that the core code is
> > sub-standard then it's you who should be fixing it.
> 
> No, there isn't this clear divide between core and the leaf
> developers.  People working on the "core" don't just grow off the
> ground.  You can't just go around piling mess around claiming that
> it's not your responsbility and then expect someone else to clean up
> after you.  How can that be *possibly* sustainable?

That's not what I said was it? You're misinterpreting me.

I'm more than happy to work on core code and have done in the past.
However, if every subsystem maintainer I submit patches to forced me
to undertake some API clean-ups/re-write or convert to another
future/development API prior to acceptance I wouldn't be able to get
any of my own work done.

> > I think it's great that forward thinking developers like Hans take on
> > challenges to improve subsystems which are lacking in one way or
> 
> Yeah, which happened only because I pushed back and Hans isn't even
> paid to do it.  Doesn't that say something?

It tells me that Hans has more spare time than I do.

This work would even be something I'd be interested in helping out
with - even in my own time, but the way you speak to people doesn't
exactly inspire them to go out of my way to work with you does it?

> Nobody is actually helping Hans' work.  Not at all, nada, zilch.
> Just complain when directed towards it.

Again, that's not what I said. It's great that your subsystem is being
improved, but insisting that anyone who submits new code to rebase
on top of some development patches which only exist in mail form, and
refusing to take patches until they do so doesn't seem right to me.

> > another, but holding back other development while this process is
> > ongoing is fundamentally wrong. Especially in this case where you're
> > still actively reviewing/NACKing the core changes.
> > 
> > Changes to APIs should either support backward-capability or change all
> > effected drivers. I haven't been able to apply the patches yet, so I
> > can't tell which one of these holds true, but I believe it's the
> > former. In which case any new driver using the _current_ (not old)
> > API should fold neatly in. I've even offered to convert to the new API
> > once it's Mainlined. How can I say fairer than that?
> > 
> > Finally, do try and stay at least a little bit professional on the
> > MLs. That sort of disrespectful, rude behaviour may be how you guys do
> > it at Redhat, but most will think it's nothing more than childish.
> 
> If you want me to be respectful towards you, don't be a crybaby
> screaming "it's not fair".  It apparently isn't clear to you that you
> have to chip in for the whole thing to be maintainable in the long
> term.  Unfair... lol, that was a good one, really.

It appears you've just regressed into childhood again. Let's continue
this when you are able to have an adult conversation which other
grown-ups.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Wed, Feb 19, 2014 at 04:39:37PM +0000, Lee Jones wrote:
> Again, that's not what I said. It's great that your subsystem is being
> improved, but insisting that anyone who submits new code to rebase
> on top of some development patches which only exist in mail form, and
> refusing to take patches until they do so doesn't seem right to me.

No policy is perfect and nothing can be decided solely on single
policy.  There of course are trade-offs to make depending on the
specific circumstances.  The problem, here, is that what has been
going on is skewed towards one extreme and has potential to develop
into a fairly large mess if left uncorrected.

The message I've been sending out has been pretty clear.  There are
multiple people duplicating about the same thing in their drivers.
Fortunately, Hans' refactoring is pretty close to completion and
should help simplifying most of them.  I'm not even asking you to do
the bulk of work.  Just take a look at it and help / push if you can.
It may be unfortunate that the circumstances haven't been completely
aligned for your convenience but that's what needs to be done to keep
things sustainable.

This is a collaborative work and what I asked you isn't some
insurmountable amount of extra work.  It's just beyond me that your
response is "it's not fair".  No wonder the whole thing has been
drifting towards mess.  That's not how this works.  Judging from your
linaro address, I assume you have been involved with some upstream
work, how can this possibly be your response?  Such attitude is
actively harmful and has no place in upstream development.

Again, of course, there can be trade-offs.  We sometimes do need to
take termporal hits in maintainability for faster hardware enablement
or whatnot; however, we can't do that without trust that the people
dumping stuff which needs later cleanups would actually help.
Unfortnately, I have close to zero trust given the recent developments
and your "it's not fair, that's not my responsibility" attitude
clearly confirms the conclusion.

So, please take long look at how you perceive upstream development.
It's a collaborative process.  Other people don't owe you by default.

-- 
tejun

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> > Again, that's not what I said. It's great that your subsystem is being
> > improved, but insisting that anyone who submits new code to rebase
> > on top of some development patches which only exist in mail form, and
> > refusing to take patches until they do so doesn't seem right to me.
> 
> No policy is perfect and nothing can be decided solely on single
> policy.  There of course are trade-offs to make depending on the
> specific circumstances.  The problem, here, is that what has been
> going on is skewed towards one extreme and has potential to develop
> into a fairly large mess if left uncorrected.
> 
> The message I've been sending out has been pretty clear.  There are
> multiple people duplicating about the same thing in their drivers.
> Fortunately, Hans' refactoring is pretty close to completion and
> should help simplifying most of them.  I'm not even asking you to do
> the bulk of work.  Just take a look at it and help / push if you can.
> It may be unfortunate that the circumstances haven't been completely
> aligned for your convenience but that's what needs to be done to keep
> things sustainable.

I understand this. Thanks for taking the time to explain properly.

FWIW, I have now managed to rebase the driver on top of Hans' work and
I am now in the process of converting it to the new way of working.

> This is a collaborative work and what I asked you isn't some
> insurmountable amount of extra work.  It's just beyond me that your
> response is "it's not fair".  No wonder the whole thing has been
> drifting towards mess.  That's not how this works.  Judging from your
> linaro address, I assume you have been involved with some upstream
> work, how can this possibly be your response?  Such attitude is
> actively harmful and has no place in upstream development.
>
> Again, of course, there can be trade-offs.  We sometimes do need to
> take termporal hits in maintainability for faster hardware enablement
> or whatnot; however, we can't do that without trust that the people
> dumping stuff which needs later cleanups would actually help.
> Unfortnately, I have close to zero trust given the recent developments
> and your "it's not fair, that's not my responsibility" attitude
> clearly confirms the conclusion.
> 
> So, please take long look at how you perceive upstream development.
> It's a collaborative process.  Other people don't owe you by default.

Please refrain from adding quotation marks around things I didn't
actually say. I didn't say that this whole process was unfair. I was
pertaining to the fact that requesting that a driver is converted to a
non-existing API was wrong. As it currently stands the driver uses the
correct one. I also said that I'd happily convert it over when the
clean-ups are actually applied.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

A few more things just in case.

On Wed, Feb 19, 2014 at 04:39:37PM +0000, Lee Jones wrote:
> It tells me that Hans has more spare time than I do.

This is the crux of the problem, isn't it?  The party who is creating
load should also partake in and invest resource into making the
infrastructure for it.  What I can't understand is how one can claim
"unfairness" at having to contribute to such effort when that is
clearly the party which is the primary beneficiary of the added load.
If you have *any* mature sense of fairness, not this childish "it's
not going my way", the irony should be clear to you.

> This work would even be something I'd be interested in helping out
> with - even in my own time, but the way you speak to people doesn't
> exactly inspire them to go out of my way to work with you does it?

Given the circumstances, I don't think depending on good wills of the
involved parties is a viable strategy and wanted to make it clear that
the responsibility of chipping in for long term maintainability is on
everyone who wants to make use of the code base.  This is beyond good
will.  It's the fundamental sharing of responsibility for
sustainability.  I'd love to have good will but I can't build that on
top of a notion as rotten as "it's not fair, it's not my
responsibility".

> Again, that's not what I said. It's great that your subsystem is being
> improved, but insisting that anyone who submits new code to rebase
> on top of some development patches which only exist in mail form, and
> refusing to take patches until they do so doesn't seem right to me.

If I apply your patch now, Hans has one more driver to worry about in
doing the work that he himself isn't directly benefiting from but
everybody needs.  In what world is that fair?

So, sorry about going f bomb on you, but you shouldn't be thinking
what you're thinking.  There's some serious misguidance going on
there.

-- 
tejun

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Lee Jones]

> A few more things just in case.
> 
> On Wed, Feb 19, 2014 at 04:39:37PM +0000, Lee Jones wrote:
> > It tells me that Hans has more spare time than I do.
> 
> This is the crux of the problem, isn't it?  The party who is creating
> load should also partake in and invest resource into making the
> infrastructure for it.  What I can't understand is how one can claim
> "unfairness" at having to contribute to such effort when that is
> clearly the party which is the primary beneficiary of the added load.
> If you have *any* mature sense of fairness, not this childish "it's
> not going my way", the irony should be clear to you.
> 
> > This work would even be something I'd be interested in helping out
> > with - even in my own time, but the way you speak to people doesn't
> > exactly inspire them to go out of my way to work with you does it?
> 
> Given the circumstances, I don't think depending on good wills of the
> involved parties is a viable strategy and wanted to make it clear that
> the responsibility of chipping in for long term maintainability is on
> everyone who wants to make use of the code base.  This is beyond good
> will.  It's the fundamental sharing of responsibility for
> sustainability.  I'd love to have good will but I can't build that on
> top of a notion as rotten as "it's not fair, it's not my
> responsibility".
> 
> > Again, that's not what I said. It's great that your subsystem is being
> > improved, but insisting that anyone who submits new code to rebase
> > on top of some development patches which only exist in mail form, and
> > refusing to take patches until they do so doesn't seem right to me.
> 
> If I apply your patch now, Hans has one more driver to worry about in
> doing the work that he himself isn't directly benefiting from but
> everybody needs.  In what world is that fair?
> 
> So, sorry about going f bomb on you, but you shouldn't be thinking
> what you're thinking.  There's some serious misguidance going on
> there.

Have you listened to a word I've said? Or have you managed to get your
self all tangled up in what you _assumed_ was meant by the 3 words I
mentioned about fairness at the very start of this conversation? Me
thinks the latter might be true.

Just because I don't contribute to your subsystem, it doesn't mean I
don't contribute elsewhere.

Here... let me help you down from that big horse! ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 3/3] ahci: st: Add support for ST's SATA IP

[Tejun Heo]

On Wed, Feb 19, 2014 at 05:40:41PM +0000, Lee Jones wrote:
> Have you listened to a word I've said? Or have you managed to get your
> self all tangled up in what you _assumed_ was meant by the 3 words I
> mentioned about fairness at the very start of this conversation? Me
> thinks the latter might be true.

It wasn't only that.  Your mention of API compability later in the
thread or the notion of separate core and leaf developers were pretty
weird.  Things like

 "I was pertaining to the fact that requesting that a driver is
  converted to a non-existing API was wrong."

just don't make much sense.  That is a patchset which is pretty close
to completion and I'm asking people to converge on that.  That happens
regularly during development in the different layers.

I think what's fundamentally disturbing is this distinction between
what you're doing and what people working on the API are doing.  It's
not like the roles are bound by contracts and one party owes to the
other.  It seems that that's what leads you to think that asking your
driver updated for a pending patchset is unfair and the rest.

> Just because I don't contribute to your subsystem, it doesn't mean I
> don't contribute elsewhere.

Again, it's not about you being good and generously contributing.
It's about sharing of basic responsibility.  You say I'm not reading
what you're saying but what you're saying hasn't really changed since
the first post except for some weaseling.

> Here... let me help you down from that big horse! ;)

I'm surely doing some of that and thanks for helping me down but I do
hope you understand some of the points.

-- 
tejun