* [PATCH 0/6] Adding support for SATA in EXYNO5
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
The following set of patches provides drivers for SATA
and SATA PHY controller and also an interface for the
two entity to interact.
Vasanth Ananthan (6):
ARM: EXYNOS5: Clock settings for SATA and SATA PHY
ARM: EXYNOS5: DT Support for SATA and SATA PHY
DRIVERS: ATA: SATA PHY utility framework
ARM: S3C2410: I2C driver polling mode support
ARM: EYNOS5: SATA controller driver
ARM: EXYNOS5: SATA PHY controller driver
arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +
arch/arm/boot/dts/exynos5250.dtsi | 20 ++
arch/arm/mach-exynos/clock-exynos5.c | 19 ++-
arch/arm/mach-exynos/include/mach/map.h | 7 +
arch/arm/mach-exynos/include/mach/regs-pmu.h | 3 +
arch/arm/mach-exynos/mach-exynos5-dt.c | 6 +
drivers/ata/Kconfig | 9 +
drivers/ata/Makefile | 1 +
drivers/ata/sata_exynos.c | 274 +++++++++++++++++++++++
drivers/ata/sata_exynos_phy.c | 303 ++++++++++++++++++++++++++
drivers/ata/sata_phy.c | 99 +++++++++
drivers/ata/sata_phy.h | 44 ++++
drivers/i2c/busses/i2c-s3c2410.c | 84 ++++++--
13 files changed, 859 insertions(+), 21 deletions(-)
create mode 100644 drivers/ata/sata_exynos.c
create mode 100644 drivers/ata/sata_exynos_phy.c
create mode 100644 drivers/ata/sata_phy.c
create mode 100644 drivers/ata/sata_phy.h
--
1.7.5.4
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/6] ARM: EXYNOS5: Clock settings for SATA and SATA PHY
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 11:48 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: kgene.kim, jgarzik, linux
Cc: linux-arm-kernel, linux-samsung-soc, linux-ide, thomas.abraham,
Vasanth Ananthan, Vasanth Ananthan
This patch adds neccessary clock entries for SATA, SATA PHY and
I2C_SATAPHY
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
arch/arm/mach-exynos/clock-exynos5.c | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-exynos/clock-exynos5.c
index 774533c..4ab964e 100644
--- a/arch/arm/mach-exynos/clock-exynos5.c
+++ b/arch/arm/mach-exynos/clock-exynos5.c
@@ -599,15 +599,20 @@ static struct clk exynos5_init_clocks_off[] = {
.ctrlbit = (1 << 16),
}, {
.name = "sata",
- .devname = "ahci",
+ .devname = "sata_exynos",
+ .parent = &exynos5_clk_aclk_200.clk,
.enable = exynos5_clk_ip_fsys_ctrl,
.ctrlbit = (1 << 6),
}, {
.name = "sata_phy",
+ .devname = "sata_phy_exynos",
+ .parent = &exynos5_clk_aclk_200.clk,
.enable = exynos5_clk_ip_fsys_ctrl,
.ctrlbit = (1 << 24),
}, {
- .name = "sata_phy_i2c",
+ .name = "i2c",
+ .devname = "sata-phy-i2c",
+ .parent = &exynos5_clk_aclk_200.clk,
.enable = exynos5_clk_ip_fsys_ctrl,
.ctrlbit = (1 << 25),
}, {
@@ -1157,6 +1162,16 @@ static struct clksrc_clk exynos5_clksrcs[] = {
.reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 24, .size = 3 },
}, {
.clk = {
+ .name = "sclk_sata",
+ .devname = "sata_exynos",
+ .enable = exynos5_clksrc_mask_fsys_ctrl,
+ .ctrlbit = (1 << 24),
+ },
+ .sources = &exynos5_clkset_aclk,
+ .reg_src = { .reg = EXYNOS5_CLKSRC_FSYS, .shift = 24, .size = 1 },
+ .reg_div = { .reg = EXYNOS5_CLKDIV_FSYS0, .shift = 20, .size = 4 },
+ }, {
+ .clk = {
.name = "sclk_gscl_wrap",
.devname = "s5p-mipi-csis.0",
.enable = exynos5_clksrc_mask_gscl_ctrl,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 1/6] ARM: EXYNOS5: Clock settings for SATA and SATA PHY
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds neccessary clock entries for SATA, SATA PHY and
I2C_SATAPHY
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
arch/arm/mach-exynos/clock-exynos5.c | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-exynos/clock-exynos5.c
index 774533c..4ab964e 100644
--- a/arch/arm/mach-exynos/clock-exynos5.c
+++ b/arch/arm/mach-exynos/clock-exynos5.c
@@ -599,15 +599,20 @@ static struct clk exynos5_init_clocks_off[] = {
.ctrlbit = (1 << 16),
}, {
.name = "sata",
- .devname = "ahci",
+ .devname = "sata_exynos",
+ .parent = &exynos5_clk_aclk_200.clk,
.enable = exynos5_clk_ip_fsys_ctrl,
.ctrlbit = (1 << 6),
}, {
.name = "sata_phy",
+ .devname = "sata_phy_exynos",
+ .parent = &exynos5_clk_aclk_200.clk,
.enable = exynos5_clk_ip_fsys_ctrl,
.ctrlbit = (1 << 24),
}, {
- .name = "sata_phy_i2c",
+ .name = "i2c",
+ .devname = "sata-phy-i2c",
+ .parent = &exynos5_clk_aclk_200.clk,
.enable = exynos5_clk_ip_fsys_ctrl,
.ctrlbit = (1 << 25),
}, {
@@ -1157,6 +1162,16 @@ static struct clksrc_clk exynos5_clksrcs[] = {
.reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 24, .size = 3 },
}, {
.clk = {
+ .name = "sclk_sata",
+ .devname = "sata_exynos",
+ .enable = exynos5_clksrc_mask_fsys_ctrl,
+ .ctrlbit = (1 << 24),
+ },
+ .sources = &exynos5_clkset_aclk,
+ .reg_src = { .reg = EXYNOS5_CLKSRC_FSYS, .shift = 24, .size = 1 },
+ .reg_div = { .reg = EXYNOS5_CLKDIV_FSYS0, .shift = 20, .size = 4 },
+ }, {
+ .clk = {
.name = "sclk_gscl_wrap",
.devname = "s5p-mipi-csis.0",
.enable = exynos5_clksrc_mask_gscl_ctrl,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 11:48 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: kgene.kim, jgarzik, linux
Cc: linux-arm-kernel, linux-samsung-soc, linux-ide, thomas.abraham,
Vasanth Ananthan, Vasanth Ananthan
This patch adds Device Nodes for SATA and SATA PHY device.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
4 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 8a5e348..bb262ce 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -48,6 +48,17 @@
};
};
+ i2c@121D0000 {
+ samsung,i2c-sda-delay = <100>;
+ samsung,i2c-max-bus-freq = <40000>;
+ samsung,i2c-slave-addr = <0x38>;
+
+ sataphy@70 {
+ compatible = "samsung,i2c-phy";
+ reg = <0x38>;
+ };
+ };
+
i2c@12C80000 {
status = "disabled";
};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 004aaa8..5a47a8f 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -88,6 +88,18 @@
interrupts = <0 54 0>;
};
+ sata@122F0000 {
+ compatible = "samsung,exynos-sata-ahci";
+ reg = <0x122F0000 0x1ff>;
+ interrupts = <0 115 0>;
+ };
+
+ sata-phy@12170000 {
+ compatible = "samsung,exynos-sata-phy";
+ reg = <0x12170000 0x1ff>;
+ };
+
+
i2c@12C60000 {
compatible = "samsung,s3c2440-i2c";
reg = <0x12C60000 0x100>;
@@ -152,6 +164,13 @@
#size-cells = <0>;
};
+ i2c@121D0000 {
+ compatible = "samsung,s3c2440-sataphy-i2c";
+ reg = <0x121D0000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
spi_0: spi@12d20000 {
compatible = "samsung,exynos4210-spi";
reg = <0x12d20000 0x100>;
@@ -460,4 +479,5 @@
#gpio-cells = <4>;
};
};
+
};
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index c72b675..6827190 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -177,9 +177,16 @@
#define EXYNOS4_PA_HSOTG 0x12480000
#define EXYNOS4_PA_USB_HSPHY 0x125B0000
+#ifdef CONFIG_ARCH_EXYNOS4
#define EXYNOS4_PA_SATA 0x12560000
#define EXYNOS4_PA_SATAPHY 0x125D0000
#define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
+#endif
+#ifdef CONFIG_ARCH_EXYNOS5
+#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
+#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
+#define EXYNOS5_PA_SATA_BASE 0x122F0000
+#endif
#define EXYNOS4_PA_SROMC 0x12570000
#define EXYNOS5_PA_SROMC 0x12250000
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c
index ef770bc..c931c6c 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -47,6 +47,12 @@ static const struct of_dev_auxdata exynos5250_auxdata_lookup[] __initconst = {
"s3c2440-i2c.0", NULL),
OF_DEV_AUXDATA("samsung,s3c2440-i2c", EXYNOS5_PA_IIC(1),
"s3c2440-i2c.1", NULL),
+ OF_DEV_AUXDATA("samsung,exynos-sata-ahci", EXYNOS5_PA_SATA_BASE,
+ "sata_exynos", NULL),
+ OF_DEV_AUXDATA("samsung,exynos-sata-phy", EXYNOS5_PA_SATA_PHY_CTRL,
+ "sata_phy_exynos", NULL),
+ OF_DEV_AUXDATA("samsung,s3c2440-sataphy-i2c", EXYNOS5_PA_SATA_PHY_I2C,
+ "sata-phy-i2c", NULL),
OF_DEV_AUXDATA("samsung,exynos4210-spi", EXYNOS5_PA_SPI0,
"exynos4210-spi.0", NULL),
OF_DEV_AUXDATA("samsung,exynos4210-spi", EXYNOS5_PA_SPI1,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds Device Nodes for SATA and SATA PHY device.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
4 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 8a5e348..bb262ce 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -48,6 +48,17 @@
};
};
+ i2c at 121D0000 {
+ samsung,i2c-sda-delay = <100>;
+ samsung,i2c-max-bus-freq = <40000>;
+ samsung,i2c-slave-addr = <0x38>;
+
+ sataphy at 70 {
+ compatible = "samsung,i2c-phy";
+ reg = <0x38>;
+ };
+ };
+
i2c at 12C80000 {
status = "disabled";
};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 004aaa8..5a47a8f 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -88,6 +88,18 @@
interrupts = <0 54 0>;
};
+ sata at 122F0000 {
+ compatible = "samsung,exynos-sata-ahci";
+ reg = <0x122F0000 0x1ff>;
+ interrupts = <0 115 0>;
+ };
+
+ sata-phy at 12170000 {
+ compatible = "samsung,exynos-sata-phy";
+ reg = <0x12170000 0x1ff>;
+ };
+
+
i2c at 12C60000 {
compatible = "samsung,s3c2440-i2c";
reg = <0x12C60000 0x100>;
@@ -152,6 +164,13 @@
#size-cells = <0>;
};
+ i2c at 121D0000 {
+ compatible = "samsung,s3c2440-sataphy-i2c";
+ reg = <0x121D0000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
spi_0: spi at 12d20000 {
compatible = "samsung,exynos4210-spi";
reg = <0x12d20000 0x100>;
@@ -460,4 +479,5 @@
#gpio-cells = <4>;
};
};
+
};
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index c72b675..6827190 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -177,9 +177,16 @@
#define EXYNOS4_PA_HSOTG 0x12480000
#define EXYNOS4_PA_USB_HSPHY 0x125B0000
+#ifdef CONFIG_ARCH_EXYNOS4
#define EXYNOS4_PA_SATA 0x12560000
#define EXYNOS4_PA_SATAPHY 0x125D0000
#define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
+#endif
+#ifdef CONFIG_ARCH_EXYNOS5
+#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
+#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
+#define EXYNOS5_PA_SATA_BASE 0x122F0000
+#endif
#define EXYNOS4_PA_SROMC 0x12570000
#define EXYNOS5_PA_SROMC 0x12250000
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c
index ef770bc..c931c6c 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -47,6 +47,12 @@ static const struct of_dev_auxdata exynos5250_auxdata_lookup[] __initconst = {
"s3c2440-i2c.0", NULL),
OF_DEV_AUXDATA("samsung,s3c2440-i2c", EXYNOS5_PA_IIC(1),
"s3c2440-i2c.1", NULL),
+ OF_DEV_AUXDATA("samsung,exynos-sata-ahci", EXYNOS5_PA_SATA_BASE,
+ "sata_exynos", NULL),
+ OF_DEV_AUXDATA("samsung,exynos-sata-phy", EXYNOS5_PA_SATA_PHY_CTRL,
+ "sata_phy_exynos", NULL),
+ OF_DEV_AUXDATA("samsung,s3c2440-sataphy-i2c", EXYNOS5_PA_SATA_PHY_I2C,
+ "sata-phy-i2c", NULL),
OF_DEV_AUXDATA("samsung,exynos4210-spi", EXYNOS5_PA_SPI0,
"exynos4210-spi.0", NULL),
OF_DEV_AUXDATA("samsung,exynos4210-spi", EXYNOS5_PA_SPI1,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 18:28 ` Olof Johansson
-1 siblings, 0 replies; 42+ messages in thread
From: Olof Johansson @ 2012-10-09 18:28 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi,
On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
> This patch adds Device Nodes for SATA and SATA PHY device.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
> arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
> arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
> arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
> 4 files changed, 44 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> index 8a5e348..bb262ce 100644
> --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> @@ -48,6 +48,17 @@
> };
> };
>
> + i2c@121D0000 {
> + samsung,i2c-sda-delay = <100>;
> + samsung,i2c-max-bus-freq = <40000>;
> + samsung,i2c-slave-addr = <0x38>;
Whitespace is off above.
> +
> + sataphy@70 {
sata-phy would be a more conventional name.
> + compatible = "samsung,i2c-phy";
i2c-phy? Seems like an odd choice of name. What is this device?
> + reg = <0x38>;
70 is unit address but here it's 0x38? One of them is wrong. No need for a unit
address if it's a unique name, by the way.
> + };
> + };
> +
> i2c@12C80000 {
> status = "disabled";
> };
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 004aaa8..5a47a8f 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -88,6 +88,18 @@
> interrupts = <0 54 0>;
> };
>
> + sata@122F0000 {
> + compatible = "samsung,exynos-sata-ahci";
> + reg = <0x122F0000 0x1ff>;
> + interrupts = <0 115 0>;
More whitespace damage. And need binding.
> + };
> +
> + sata-phy@12170000 {
> + compatible = "samsung,exynos-sata-phy";
> + reg = <0x12170000 0x1ff>;
> + };
Should have binding too. How does this relate to the i2c device above.
> +
> i2c@12C60000 {
> compatible = "samsung,s3c2440-i2c";
> reg = <0x12C60000 0x100>;
> @@ -152,6 +164,13 @@
> #size-cells = <0>;
> };
>
> + i2c@121D0000 {
> + compatible = "samsung,s3c2440-sataphy-i2c";
Is this a unique i2c controller, or is it just another one like the others on
the chip? If it's the latter, it should use the regular compatible string.
> + reg = <0x121D0000 0x100>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> spi_0: spi@12d20000 {
> compatible = "samsung,exynos4210-spi";
> reg = <0x12d20000 0x100>;
> @@ -460,4 +479,5 @@
> #gpio-cells = <4>;
> };
> };
> +
Stray whitespace change.
> };
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index c72b675..6827190 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -177,9 +177,16 @@
> #define EXYNOS4_PA_HSOTG 0x12480000
> #define EXYNOS4_PA_USB_HSPHY 0x125B0000
>
> +#ifdef CONFIG_ARCH_EXYNOS4
No need to ifdef since namespace isn't overlapped.
> #define EXYNOS4_PA_SATA 0x12560000
> #define EXYNOS4_PA_SATAPHY 0x125D0000
> #define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
> +#endif
> +#ifdef CONFIG_ARCH_EXYNOS5
Same here.
> +#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
> +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
> +#define EXYNOS5_PA_SATA_BASE 0x122F0000
> +#endif
-Olof
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
@ 2012-10-09 18:28 ` Olof Johansson
0 siblings, 0 replies; 42+ messages in thread
From: Olof Johansson @ 2012-10-09 18:28 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
> This patch adds Device Nodes for SATA and SATA PHY device.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
> arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
> arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
> arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
> 4 files changed, 44 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> index 8a5e348..bb262ce 100644
> --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> @@ -48,6 +48,17 @@
> };
> };
>
> + i2c at 121D0000 {
> + samsung,i2c-sda-delay = <100>;
> + samsung,i2c-max-bus-freq = <40000>;
> + samsung,i2c-slave-addr = <0x38>;
Whitespace is off above.
> +
> + sataphy at 70 {
sata-phy would be a more conventional name.
> + compatible = "samsung,i2c-phy";
i2c-phy? Seems like an odd choice of name. What is this device?
> + reg = <0x38>;
70 is unit address but here it's 0x38? One of them is wrong. No need for a unit
address if it's a unique name, by the way.
> + };
> + };
> +
> i2c at 12C80000 {
> status = "disabled";
> };
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 004aaa8..5a47a8f 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -88,6 +88,18 @@
> interrupts = <0 54 0>;
> };
>
> + sata at 122F0000 {
> + compatible = "samsung,exynos-sata-ahci";
> + reg = <0x122F0000 0x1ff>;
> + interrupts = <0 115 0>;
More whitespace damage. And need binding.
> + };
> +
> + sata-phy at 12170000 {
> + compatible = "samsung,exynos-sata-phy";
> + reg = <0x12170000 0x1ff>;
> + };
Should have binding too. How does this relate to the i2c device above.
> +
> i2c at 12C60000 {
> compatible = "samsung,s3c2440-i2c";
> reg = <0x12C60000 0x100>;
> @@ -152,6 +164,13 @@
> #size-cells = <0>;
> };
>
> + i2c at 121D0000 {
> + compatible = "samsung,s3c2440-sataphy-i2c";
Is this a unique i2c controller, or is it just another one like the others on
the chip? If it's the latter, it should use the regular compatible string.
> + reg = <0x121D0000 0x100>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> spi_0: spi at 12d20000 {
> compatible = "samsung,exynos4210-spi";
> reg = <0x12d20000 0x100>;
> @@ -460,4 +479,5 @@
> #gpio-cells = <4>;
> };
> };
> +
Stray whitespace change.
> };
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index c72b675..6827190 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -177,9 +177,16 @@
> #define EXYNOS4_PA_HSOTG 0x12480000
> #define EXYNOS4_PA_USB_HSPHY 0x125B0000
>
> +#ifdef CONFIG_ARCH_EXYNOS4
No need to ifdef since namespace isn't overlapped.
> #define EXYNOS4_PA_SATA 0x12560000
> #define EXYNOS4_PA_SATAPHY 0x125D0000
> #define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
> +#endif
> +#ifdef CONFIG_ARCH_EXYNOS5
Same here.
> +#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
> +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
> +#define EXYNOS5_PA_SATA_BASE 0x122F0000
> +#endif
-Olof
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
2012-10-09 18:28 ` Olof Johansson
@ 2012-10-10 8:38 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-10 8:38 UTC (permalink / raw)
To: Olof Johansson
Cc: linux-samsung-soc, linux, Vasanth Ananthan, linux-ide, kgene.kim,
thomas.abraham, jgarzik, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 4961 bytes --]
Hi,
On Tue, Oct 9, 2012 at 11:58 PM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
>
> On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
> > This patch adds Device Nodes for SATA and SATA PHY device.
> >
> > Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> > ---
> > arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
> > arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
> > arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
> > arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
> > 4 files changed, 44 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > index 8a5e348..bb262ce 100644
> > --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > @@ -48,6 +48,17 @@
> > };
> > };
> >
> > + i2c@121D0000 {
> > + samsung,i2c-sda-delay = <100>;
> > + samsung,i2c-max-bus-freq = <40000>;
> > + samsung,i2c-slave-addr = <0x38>;
>
> Whitespace is off above.
>
> > +
> > + sataphy@70 {
>
> sata-phy would be a more conventional name.
>
> > + compatible = "samsung,i2c-phy";
>
> i2c-phy? Seems like an odd choice of name. What is this device?
>
The SATA physical layer controller is both a platform device and a i2c
slave device.
This compatible string is for the i2c client driver. Hence I have used
i2c-phy here.
>
> > + reg = <0x38>;
>
> 70 is unit address but here it's 0x38? One of them is wrong. No need for a
> unit
> address if it's a unique name, by the way.
>
>
> > + };
> > + };
> > +
> > i2c@12C80000 {
> > status = "disabled";
> > };
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> > index 004aaa8..5a47a8f 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -88,6 +88,18 @@
> > interrupts = <0 54 0>;
> > };
> >
> > + sata@122F0000 {
> > + compatible = "samsung,exynos-sata-ahci";
> > + reg = <0x122F0000 0x1ff>;
> > + interrupts = <0 115 0>;
>
> More whitespace damage. And need binding.
>
> > + };
> > +
> > + sata-phy@12170000 {
> > + compatible = "samsung,exynos-sata-phy";
> > + reg = <0x12170000 0x1ff>;
> > + };
>
> Should have binding too. How does this relate to the i2c device above.
>
As mentioned earlier SATA physical layer controller is both a platform
device and also an i2c slave device.
This Node is for the SATA physical layer platform device, the previous node
is for i2c slave device.
Certain initialization settings done directly and other settings has to be
done through i2c.
> > +
> > i2c@12C60000 {
> > compatible = "samsung,s3c2440-i2c";
> > reg = <0x12C60000 0x100>;
> > @@ -152,6 +164,13 @@
> > #size-cells = <0>;
> > };
> >
> > + i2c@121D0000 {
> > + compatible = "samsung,s3c2440-sataphy-i2c";
>
> Is this a unique i2c controller, or is it just another one like the others
> on
> the chip? If it's the latter, it should use the regular compatible string.
>
Yes, its a unique i2c controller which lacks an interrupt line while others
are interrupt driven.
Hence I have used a distinct compatible string for the driver to
distinguish the controller.
> > + reg = <0x121D0000 0x100>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > spi_0: spi@12d20000 {
> > compatible = "samsung,exynos4210-spi";
> > reg = <0x12d20000 0x100>;
> > @@ -460,4 +479,5 @@
> > #gpio-cells = <4>;
> > };
> > };
> > +
>
> Stray whitespace change.
>
> > };
> > diff --git a/arch/arm/mach-exynos/include/mach/map.h
> b/arch/arm/mach-exynos/include/mach/map.h
> > index c72b675..6827190 100644
> > --- a/arch/arm/mach-exynos/include/mach/map.h
> > +++ b/arch/arm/mach-exynos/include/mach/map.h
> > @@ -177,9 +177,16 @@
> > #define EXYNOS4_PA_HSOTG 0x12480000
> > #define EXYNOS4_PA_USB_HSPHY 0x125B0000
> >
> > +#ifdef CONFIG_ARCH_EXYNOS4
>
> No need to ifdef since namespace isn't overlapped.
>
> > #define EXYNOS4_PA_SATA 0x12560000
> > #define EXYNOS4_PA_SATAPHY 0x125D0000
> > #define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
> > +#endif
> > +#ifdef CONFIG_ARCH_EXYNOS5
>
> Same here.
>
> > +#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
> > +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
> > +#define EXYNOS5_PA_SATA_BASE 0x122F0000
> > +#endif
>
>
> -Olof
>
I will incorporate the other comments and resubmit the patch. Thanks.
--
Vasanth Ananthan.
[-- Attachment #1.2: Type: text/html, Size: 6820 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
@ 2012-10-10 8:38 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-10 8:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Oct 9, 2012 at 11:58 PM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
>
> On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
> > This patch adds Device Nodes for SATA and SATA PHY device.
> >
> > Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> > ---
> > arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
> > arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
> > arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
> > arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
> > 4 files changed, 44 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > index 8a5e348..bb262ce 100644
> > --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > @@ -48,6 +48,17 @@
> > };
> > };
> >
> > + i2c at 121D0000 {
> > + samsung,i2c-sda-delay = <100>;
> > + samsung,i2c-max-bus-freq = <40000>;
> > + samsung,i2c-slave-addr = <0x38>;
>
> Whitespace is off above.
>
> > +
> > + sataphy at 70 {
>
> sata-phy would be a more conventional name.
>
> > + compatible = "samsung,i2c-phy";
>
> i2c-phy? Seems like an odd choice of name. What is this device?
>
The SATA physical layer controller is both a platform device and a i2c
slave device.
This compatible string is for the i2c client driver. Hence I have used
i2c-phy here.
>
> > + reg = <0x38>;
>
> 70 is unit address but here it's 0x38? One of them is wrong. No need for a
> unit
> address if it's a unique name, by the way.
>
>
> > + };
> > + };
> > +
> > i2c at 12C80000 {
> > status = "disabled";
> > };
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> > index 004aaa8..5a47a8f 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -88,6 +88,18 @@
> > interrupts = <0 54 0>;
> > };
> >
> > + sata at 122F0000 {
> > + compatible = "samsung,exynos-sata-ahci";
> > + reg = <0x122F0000 0x1ff>;
> > + interrupts = <0 115 0>;
>
> More whitespace damage. And need binding.
>
> > + };
> > +
> > + sata-phy at 12170000 {
> > + compatible = "samsung,exynos-sata-phy";
> > + reg = <0x12170000 0x1ff>;
> > + };
>
> Should have binding too. How does this relate to the i2c device above.
>
As mentioned earlier SATA physical layer controller is both a platform
device and also an i2c slave device.
This Node is for the SATA physical layer platform device, the previous node
is for i2c slave device.
Certain initialization settings done directly and other settings has to be
done through i2c.
> > +
> > i2c at 12C60000 {
> > compatible = "samsung,s3c2440-i2c";
> > reg = <0x12C60000 0x100>;
> > @@ -152,6 +164,13 @@
> > #size-cells = <0>;
> > };
> >
> > + i2c at 121D0000 {
> > + compatible = "samsung,s3c2440-sataphy-i2c";
>
> Is this a unique i2c controller, or is it just another one like the others
> on
> the chip? If it's the latter, it should use the regular compatible string.
>
Yes, its a unique i2c controller which lacks an interrupt line while others
are interrupt driven.
Hence I have used a distinct compatible string for the driver to
distinguish the controller.
> > + reg = <0x121D0000 0x100>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > spi_0: spi at 12d20000 {
> > compatible = "samsung,exynos4210-spi";
> > reg = <0x12d20000 0x100>;
> > @@ -460,4 +479,5 @@
> > #gpio-cells = <4>;
> > };
> > };
> > +
>
> Stray whitespace change.
>
> > };
> > diff --git a/arch/arm/mach-exynos/include/mach/map.h
> b/arch/arm/mach-exynos/include/mach/map.h
> > index c72b675..6827190 100644
> > --- a/arch/arm/mach-exynos/include/mach/map.h
> > +++ b/arch/arm/mach-exynos/include/mach/map.h
> > @@ -177,9 +177,16 @@
> > #define EXYNOS4_PA_HSOTG 0x12480000
> > #define EXYNOS4_PA_USB_HSPHY 0x125B0000
> >
> > +#ifdef CONFIG_ARCH_EXYNOS4
>
> No need to ifdef since namespace isn't overlapped.
>
> > #define EXYNOS4_PA_SATA 0x12560000
> > #define EXYNOS4_PA_SATAPHY 0x125D0000
> > #define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
> > +#endif
> > +#ifdef CONFIG_ARCH_EXYNOS5
>
> Same here.
>
> > +#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
> > +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
> > +#define EXYNOS5_PA_SATA_BASE 0x122F0000
> > +#endif
>
>
> -Olof
>
I will incorporate the other comments and resubmit the patch. Thanks.
--
Vasanth Ananthan.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121010/7856e75d/attachment-0001.html>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
2012-10-10 8:38 ` Vasanth Ananthan
@ 2012-10-10 9:20 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-10 9:20 UTC (permalink / raw)
To: Olof Johansson; +Cc: linux-samsung-soc, linux-ide, linux-arm-kernel
Hi,
On Tue, Oct 9, 2012 at 11:58 PM, Olof Johansson <olof@lixom.net> wrote:
>
> Hi,
>
>
> On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
> > This patch adds Device Nodes for SATA and SATA PHY device.
> >
> > Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> > ---
> > arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
> > arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
> > arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
> > arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
> > 4 files changed, 44 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > index 8a5e348..bb262ce 100644
> > --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > @@ -48,6 +48,17 @@
> > };
> > };
> >
> > + i2c@121D0000 {
> > + samsung,i2c-sda-delay = <100>;
> > + samsung,i2c-max-bus-freq = <40000>;
> > + samsung,i2c-slave-addr = <0x38>;
>
> Whitespace is off above.
>
> > +
> > + sataphy@70 {
>
> sata-phy would be a more conventional name.
>
> > + compatible = "samsung,i2c-phy";
>
> i2c-phy? Seems like an odd choice of name. What is this device?
The SATA physical layer controller is both a platform device and a i2c
slave device.
This compatible string is for the i2c client driver. Hence I have used
i2c-phy here.
>
>
> > + reg = <0x38>;
>
> 70 is unit address but here it's 0x38? One of them is wrong. No need for a unit
> address if it's a unique name, by the way.
>
>
> > + };
> > + };
> > +
> > i2c@12C80000 {
> > status = "disabled";
> > };
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> > index 004aaa8..5a47a8f 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -88,6 +88,18 @@
> > interrupts = <0 54 0>;
> > };
> >
> > + sata@122F0000 {
> > + compatible = "samsung,exynos-sata-ahci";
> > + reg = <0x122F0000 0x1ff>;
> > + interrupts = <0 115 0>;
>
> More whitespace damage. And need binding.
>
> > + };
> > +
> > + sata-phy@12170000 {
> > + compatible = "samsung,exynos-sata-phy";
> > + reg = <0x12170000 0x1ff>;
> > + };
>
> Should have binding too. How does this relate to the i2c device above.
As mentioned earlier SATA physical layer controller is both a platform
device and also an i2c slave device.
This Node is for the SATA physical layer platform device, the previous
node is for i2c slave device.
Certain initialization settings done directly and other settings has
to be done through i2c.
>
> > +
> > i2c@12C60000 {
> > compatible = "samsung,s3c2440-i2c";
> > reg = <0x12C60000 0x100>;
> > @@ -152,6 +164,13 @@
> > #size-cells = <0>;
> > };
> >
> > + i2c@121D0000 {
> > + compatible = "samsung,s3c2440-sataphy-i2c";
>
> Is this a unique i2c controller, or is it just another one like the others on
> the chip? If it's the latter, it should use the regular compatible string.
Yes, its a unique i2c controller which lacks an interrupt line while
others are interrupt driven.
Hence I have used a distinct compatible string for the driver to
distinguish the controller.
>
> > + reg = <0x121D0000 0x100>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > spi_0: spi@12d20000 {
> > compatible = "samsung,exynos4210-spi";
> > reg = <0x12d20000 0x100>;
> > @@ -460,4 +479,5 @@
> > #gpio-cells = <4>;
> > };
> > };
> > +
>
> Stray whitespace change.
>
> > };
> > diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> > index c72b675..6827190 100644
> > --- a/arch/arm/mach-exynos/include/mach/map.h
> > +++ b/arch/arm/mach-exynos/include/mach/map.h
> > @@ -177,9 +177,16 @@
> > #define EXYNOS4_PA_HSOTG 0x12480000
> > #define EXYNOS4_PA_USB_HSPHY 0x125B0000
> >
> > +#ifdef CONFIG_ARCH_EXYNOS4
>
> No need to ifdef since namespace isn't overlapped.
>
> > #define EXYNOS4_PA_SATA 0x12560000
> > #define EXYNOS4_PA_SATAPHY 0x125D0000
> > #define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
> > +#endif
> > +#ifdef CONFIG_ARCH_EXYNOS5
>
> Same here.
>
> > +#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
> > +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
> > +#define EXYNOS5_PA_SATA_BASE 0x122F0000
> > +#endif
>
>
> -Olof
I will incorporate the other comments and resubmit the patch. Thanks.
--
Vasanth Ananthan.
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
@ 2012-10-10 9:20 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-10 9:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Oct 9, 2012 at 11:58 PM, Olof Johansson <olof@lixom.net> wrote:
>
> Hi,
>
>
> On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
> > This patch adds Device Nodes for SATA and SATA PHY device.
> >
> > Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> > ---
> > arch/arm/boot/dts/exynos5250-smdk5250.dts | 11 +++++++++++
> > arch/arm/boot/dts/exynos5250.dtsi | 20 ++++++++++++++++++++
> > arch/arm/mach-exynos/include/mach/map.h | 7 +++++++
> > arch/arm/mach-exynos/mach-exynos5-dt.c | 6 ++++++
> > 4 files changed, 44 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > index 8a5e348..bb262ce 100644
> > --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> > @@ -48,6 +48,17 @@
> > };
> > };
> >
> > + i2c at 121D0000 {
> > + samsung,i2c-sda-delay = <100>;
> > + samsung,i2c-max-bus-freq = <40000>;
> > + samsung,i2c-slave-addr = <0x38>;
>
> Whitespace is off above.
>
> > +
> > + sataphy at 70 {
>
> sata-phy would be a more conventional name.
>
> > + compatible = "samsung,i2c-phy";
>
> i2c-phy? Seems like an odd choice of name. What is this device?
The SATA physical layer controller is both a platform device and a i2c
slave device.
This compatible string is for the i2c client driver. Hence I have used
i2c-phy here.
>
>
> > + reg = <0x38>;
>
> 70 is unit address but here it's 0x38? One of them is wrong. No need for a unit
> address if it's a unique name, by the way.
>
>
> > + };
> > + };
> > +
> > i2c at 12C80000 {
> > status = "disabled";
> > };
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> > index 004aaa8..5a47a8f 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -88,6 +88,18 @@
> > interrupts = <0 54 0>;
> > };
> >
> > + sata at 122F0000 {
> > + compatible = "samsung,exynos-sata-ahci";
> > + reg = <0x122F0000 0x1ff>;
> > + interrupts = <0 115 0>;
>
> More whitespace damage. And need binding.
>
> > + };
> > +
> > + sata-phy at 12170000 {
> > + compatible = "samsung,exynos-sata-phy";
> > + reg = <0x12170000 0x1ff>;
> > + };
>
> Should have binding too. How does this relate to the i2c device above.
As mentioned earlier SATA physical layer controller is both a platform
device and also an i2c slave device.
This Node is for the SATA physical layer platform device, the previous
node is for i2c slave device.
Certain initialization settings done directly and other settings has
to be done through i2c.
>
> > +
> > i2c at 12C60000 {
> > compatible = "samsung,s3c2440-i2c";
> > reg = <0x12C60000 0x100>;
> > @@ -152,6 +164,13 @@
> > #size-cells = <0>;
> > };
> >
> > + i2c at 121D0000 {
> > + compatible = "samsung,s3c2440-sataphy-i2c";
>
> Is this a unique i2c controller, or is it just another one like the others on
> the chip? If it's the latter, it should use the regular compatible string.
Yes, its a unique i2c controller which lacks an interrupt line while
others are interrupt driven.
Hence I have used a distinct compatible string for the driver to
distinguish the controller.
>
> > + reg = <0x121D0000 0x100>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > spi_0: spi at 12d20000 {
> > compatible = "samsung,exynos4210-spi";
> > reg = <0x12d20000 0x100>;
> > @@ -460,4 +479,5 @@
> > #gpio-cells = <4>;
> > };
> > };
> > +
>
> Stray whitespace change.
>
> > };
> > diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> > index c72b675..6827190 100644
> > --- a/arch/arm/mach-exynos/include/mach/map.h
> > +++ b/arch/arm/mach-exynos/include/mach/map.h
> > @@ -177,9 +177,16 @@
> > #define EXYNOS4_PA_HSOTG 0x12480000
> > #define EXYNOS4_PA_USB_HSPHY 0x125B0000
> >
> > +#ifdef CONFIG_ARCH_EXYNOS4
>
> No need to ifdef since namespace isn't overlapped.
>
> > #define EXYNOS4_PA_SATA 0x12560000
> > #define EXYNOS4_PA_SATAPHY 0x125D0000
> > #define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000
> > +#endif
> > +#ifdef CONFIG_ARCH_EXYNOS5
>
> Same here.
>
> > +#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000
> > +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000
> > +#define EXYNOS5_PA_SATA_BASE 0x122F0000
> > +#endif
>
>
> -Olof
I will incorporate the other comments and resubmit the patch. Thanks.
--
Vasanth Ananthan.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
2012-10-10 8:38 ` Vasanth Ananthan
@ 2012-10-10 16:31 ` Olof Johansson
-1 siblings, 0 replies; 42+ messages in thread
From: Olof Johansson @ 2012-10-10 16:31 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi,
On Wed, Oct 10, 2012 at 02:08:31PM +0530, Vasanth Ananthan wrote:
> > > +
> > > + sataphy@70 {
> >
> > sata-phy would be a more conventional name.
> >
> > > + compatible = "samsung,i2c-phy";
> >
> > i2c-phy? Seems like an odd choice of name. What is this device?
> >
>
> The SATA physical layer controller is both a platform device and a i2c
> slave device.
> This compatible string is for the i2c client driver. Hence I have used
> i2c-phy here.
Ok, but samsung,i2c-phy is much too generic. Maybe something like
samsung,exynos5-sata-phy (and rename the MMIO-controlled phy controls
to ...sata-phy-controller below).
> > > + };
> > > +
> > > + sata-phy@12170000 {
> > > + compatible = "samsung,exynos-sata-phy";
> > > + reg = <0x12170000 0x1ff>;
> > > + };
> >
> > Should have binding too. How does this relate to the i2c device above.
> >
>
> As mentioned earlier SATA physical layer controller is both a platform
> device and also an i2c slave device.
> This Node is for the SATA physical layer platform device, the previous node
> is for i2c slave device.
> Certain initialization settings done directly and other settings has to be
> done through i2c.
Wow, that's quite awkward. What needs to be done over i2c? I think I have
seen use of SATA without touching the i2c side but it might have been for
a simple setup.
> > > +
> > > i2c@12C60000 {
> > > compatible = "samsung,s3c2440-i2c";
> > > reg = <0x12C60000 0x100>;
> > > @@ -152,6 +164,13 @@
> > > #size-cells = <0>;
> > > };
> > >
> > > + i2c@121D0000 {
> > > + compatible = "samsung,s3c2440-sataphy-i2c";
> >
> > Is this a unique i2c controller, or is it just another one like the others
> > on
> > the chip? If it's the latter, it should use the regular compatible string.
> >
>
> Yes, its a unique i2c controller which lacks an interrupt line while others
> are interrupt driven.
> Hence I have used a distinct compatible string for the driver to
> distinguish the controller.
It would be better to just make the i2c bus driver handle the case where there
is no interrupt specifier and just use polling in those cases, especially if
the rest of the device is identical and doesn't need special handling.
As a matter of fact, if that had been done for the hdmi phy, then you could
have done this patch without modifying the driver at all, just device tree
contents. And the same would go for the next time down the road when
a "special" i2c bus is added.
-Olof
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
@ 2012-10-10 16:31 ` Olof Johansson
0 siblings, 0 replies; 42+ messages in thread
From: Olof Johansson @ 2012-10-10 16:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Oct 10, 2012 at 02:08:31PM +0530, Vasanth Ananthan wrote:
> > > +
> > > + sataphy at 70 {
> >
> > sata-phy would be a more conventional name.
> >
> > > + compatible = "samsung,i2c-phy";
> >
> > i2c-phy? Seems like an odd choice of name. What is this device?
> >
>
> The SATA physical layer controller is both a platform device and a i2c
> slave device.
> This compatible string is for the i2c client driver. Hence I have used
> i2c-phy here.
Ok, but samsung,i2c-phy is much too generic. Maybe something like
samsung,exynos5-sata-phy (and rename the MMIO-controlled phy controls
to ...sata-phy-controller below).
> > > + };
> > > +
> > > + sata-phy at 12170000 {
> > > + compatible = "samsung,exynos-sata-phy";
> > > + reg = <0x12170000 0x1ff>;
> > > + };
> >
> > Should have binding too. How does this relate to the i2c device above.
> >
>
> As mentioned earlier SATA physical layer controller is both a platform
> device and also an i2c slave device.
> This Node is for the SATA physical layer platform device, the previous node
> is for i2c slave device.
> Certain initialization settings done directly and other settings has to be
> done through i2c.
Wow, that's quite awkward. What needs to be done over i2c? I think I have
seen use of SATA without touching the i2c side but it might have been for
a simple setup.
> > > +
> > > i2c at 12C60000 {
> > > compatible = "samsung,s3c2440-i2c";
> > > reg = <0x12C60000 0x100>;
> > > @@ -152,6 +164,13 @@
> > > #size-cells = <0>;
> > > };
> > >
> > > + i2c at 121D0000 {
> > > + compatible = "samsung,s3c2440-sataphy-i2c";
> >
> > Is this a unique i2c controller, or is it just another one like the others
> > on
> > the chip? If it's the latter, it should use the regular compatible string.
> >
>
> Yes, its a unique i2c controller which lacks an interrupt line while others
> are interrupt driven.
> Hence I have used a distinct compatible string for the driver to
> distinguish the controller.
It would be better to just make the i2c bus driver handle the case where there
is no interrupt specifier and just use polling in those cases, especially if
the rest of the device is identical and doesn't need special handling.
As a matter of fact, if that had been done for the hdmi phy, then you could
have done this patch without modifying the driver at all, just device tree
contents. And the same would go for the next time down the road when
a "special" i2c bus is added.
-Olof
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
2012-10-10 16:31 ` Olof Johansson
@ 2012-10-11 6:49 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-11 6:49 UTC (permalink / raw)
To: Olof Johansson
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi,
On Wed, Oct 10, 2012 at 10:01 PM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> On Wed, Oct 10, 2012 at 02:08:31PM +0530, Vasanth Ananthan wrote:
>
>> > > +
>> > > + sataphy@70 {
>> >
>> > sata-phy would be a more conventional name.
>> >
>> > > + compatible = "samsung,i2c-phy";
>> >
>> > i2c-phy? Seems like an odd choice of name. What is this device?
>> >
>>
>> The SATA physical layer controller is both a platform device and a i2c
>> slave device.
>> This compatible string is for the i2c client driver. Hence I have used
>> i2c-phy here.
>
> Ok, but samsung,i2c-phy is much too generic. Maybe something like
> samsung,exynos5-sata-phy (and rename the MMIO-controlled phy controls
> to ...sata-phy-controller below).
>
I'll do so..
>
>> > > + };
>> > > +
>> > > + sata-phy@12170000 {
>> > > + compatible = "samsung,exynos-sata-phy";
>> > > + reg = <0x12170000 0x1ff>;
>> > > + };
>> >
>> > Should have binding too. How does this relate to the i2c device above.
>> >
>>
>> As mentioned earlier SATA physical layer controller is both a platform
>> device and also an i2c slave device.
>> This Node is for the SATA physical layer platform device, the previous node
>> is for i2c slave device.
>> Certain initialization settings done directly and other settings has to be
>> done through i2c.
>
> Wow, that's quite awkward. What needs to be done over i2c? I think I have
> seen use of SATA without touching the i2c side but it might have been for
> a simple setup.
40 bit Interface setting is done through i2c. Internal setting address
0x3A and data 0x0B.
>
>> > > +
>> > > i2c@12C60000 {
>> > > compatible = "samsung,s3c2440-i2c";
>> > > reg = <0x12C60000 0x100>;
>> > > @@ -152,6 +164,13 @@
>> > > #size-cells = <0>;
>> > > };
>> > >
>> > > + i2c@121D0000 {
>> > > + compatible = "samsung,s3c2440-sataphy-i2c";
>> >
>> > Is this a unique i2c controller, or is it just another one like the others
>> > on
>> > the chip? If it's the latter, it should use the regular compatible string.
>> >
>>
>> Yes, its a unique i2c controller which lacks an interrupt line while others
>> are interrupt driven.
>> Hence I have used a distinct compatible string for the driver to
>> distinguish the controller.
>
> It would be better to just make the i2c bus driver handle the case where there
> is no interrupt specifier and just use polling in those cases, especially if
> the rest of the device is identical and doesn't need special handling.
>
> As a matter of fact, if that had been done for the hdmi phy, then you could
> have done this patch without modifying the driver at all, just device tree
> contents. And the same would go for the next time down the road when
> a "special" i2c bus is added.
>
Yes, It is the i2c bus driver thats handles this case. There is a
subsequent patch
that provides the polling support to the driver. As far as I know, the
i2c controller for
HDMI PHY is also interrupt driven.
>
> -Olof
--
Regards,
Vasanth K A
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY
@ 2012-10-11 6:49 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-11 6:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Oct 10, 2012 at 10:01 PM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> On Wed, Oct 10, 2012 at 02:08:31PM +0530, Vasanth Ananthan wrote:
>
>> > > +
>> > > + sataphy at 70 {
>> >
>> > sata-phy would be a more conventional name.
>> >
>> > > + compatible = "samsung,i2c-phy";
>> >
>> > i2c-phy? Seems like an odd choice of name. What is this device?
>> >
>>
>> The SATA physical layer controller is both a platform device and a i2c
>> slave device.
>> This compatible string is for the i2c client driver. Hence I have used
>> i2c-phy here.
>
> Ok, but samsung,i2c-phy is much too generic. Maybe something like
> samsung,exynos5-sata-phy (and rename the MMIO-controlled phy controls
> to ...sata-phy-controller below).
>
I'll do so..
>
>> > > + };
>> > > +
>> > > + sata-phy at 12170000 {
>> > > + compatible = "samsung,exynos-sata-phy";
>> > > + reg = <0x12170000 0x1ff>;
>> > > + };
>> >
>> > Should have binding too. How does this relate to the i2c device above.
>> >
>>
>> As mentioned earlier SATA physical layer controller is both a platform
>> device and also an i2c slave device.
>> This Node is for the SATA physical layer platform device, the previous node
>> is for i2c slave device.
>> Certain initialization settings done directly and other settings has to be
>> done through i2c.
>
> Wow, that's quite awkward. What needs to be done over i2c? I think I have
> seen use of SATA without touching the i2c side but it might have been for
> a simple setup.
40 bit Interface setting is done through i2c. Internal setting address
0x3A and data 0x0B.
>
>> > > +
>> > > i2c at 12C60000 {
>> > > compatible = "samsung,s3c2440-i2c";
>> > > reg = <0x12C60000 0x100>;
>> > > @@ -152,6 +164,13 @@
>> > > #size-cells = <0>;
>> > > };
>> > >
>> > > + i2c at 121D0000 {
>> > > + compatible = "samsung,s3c2440-sataphy-i2c";
>> >
>> > Is this a unique i2c controller, or is it just another one like the others
>> > on
>> > the chip? If it's the latter, it should use the regular compatible string.
>> >
>>
>> Yes, its a unique i2c controller which lacks an interrupt line while others
>> are interrupt driven.
>> Hence I have used a distinct compatible string for the driver to
>> distinguish the controller.
>
> It would be better to just make the i2c bus driver handle the case where there
> is no interrupt specifier and just use polling in those cases, especially if
> the rest of the device is identical and doesn't need special handling.
>
> As a matter of fact, if that had been done for the hdmi phy, then you could
> have done this patch without modifying the driver at all, just device tree
> contents. And the same would go for the next time down the road when
> a "special" i2c bus is added.
>
Yes, It is the i2c bus driver thats handles this case. There is a
subsequent patch
that provides the polling support to the driver. As far as I know, the
i2c controller for
HDMI PHY is also interrupt driven.
>
> -Olof
--
Regards,
Vasanth K A
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/6] DRIVERS: ATA: SATA PHY utility framework
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 11:48 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: kgene.kim, jgarzik, linux
Cc: linux-arm-kernel, linux-samsung-soc, linux-ide, thomas.abraham,
Vasanth Ananthan, Vasanth Ananthan
This patch adds SATA PHY utility framework APIs. The framework acts as an
interface between the SATA device and the PHY device. The SATA PHY device
registers itself with the framework through the APIs provided and the SATA
device finds and requests for an appropriate PHY device.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
drivers/ata/Kconfig | 9 ++++
drivers/ata/Makefile | 1 +
drivers/ata/sata_phy.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/sata_phy.h | 44 +++++++++++++++++++++
4 files changed, 153 insertions(+), 0 deletions(-)
create mode 100644 drivers/ata/sata_phy.c
create mode 100644 drivers/ata/sata_phy.h
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 27cecd3..0344b78 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -83,6 +83,15 @@ config SATA_AHCI_PLATFORM
If unsure, say N.
+config SATA_EXYNOS
+ bool "Exynos SATA AHCI support"
+ depends on I2C_S3C2410
+ help
+ This option enables support for Exynos AHCI Serial ATA
+ controllers.
+
+ If unsure, say N.
+
config SATA_FSL
tristate "Freescale 3.0Gbps SATA support"
depends on FSL_SOC
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index a454a13..bf3fd91 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
+obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/sata_phy.c b/drivers/ata/sata_phy.c
new file mode 100644
index 0000000..dbb4aa3
--- /dev/null
+++ b/drivers/ata/sata_phy.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA utility framework.
+ *
+ * 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/kernel.h>
+#include <linux/export.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include "sata_phy.h"
+
+static LIST_HEAD(phy_list);
+static DEFINE_SPINLOCK(phy_lock);
+
+struct sata_phy *sata_get_phy(enum sata_phy_type type)
+{
+ struct sata_phy *x = NULL;
+ unsigned long flag;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ list_for_each_entry(x, &phy_list, head) {
+ if (x->type == type) {
+ get_device(x->dev);
+ break;
+ }
+ }
+
+ spin_unlock_irqrestore(&phy_lock, flag);
+ return x;
+}
+EXPORT_SYMBOL(sata_get_phy);
+
+int sata_add_phy(struct sata_phy *phy, enum sata_phy_type type)
+{
+ unsigned long flag;
+ unsigned int ret = -EINVAL;
+ struct sata_phy *x;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ if (phy) {
+
+ list_for_each_entry(x, &phy_list, head) {
+ if (x->type == type) {
+ dev_err(phy->dev, "transceiver type already exists\n");
+ goto out;
+ }
+ }
+ phy->type = type;
+ list_add_tail(&phy->head, &phy_list);
+ ret = 0;
+ }
+
+ out:
+ spin_unlock_irqrestore(&phy_lock, flag);
+ return ret;
+}
+EXPORT_SYMBOL(sata_add_phy);
+
+void sata_remove_phy(struct sata_phy *phy)
+{
+ unsigned long flag;
+ struct sata_phy *x;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ if (phy) {
+ list_for_each_entry(x, &phy_list, head) {
+ if (x->type == phy->type)
+ list_del(&phy->head);
+ }
+ }
+
+ spin_unlock_irqrestore(&phy_lock, flag);
+}
+EXPORT_SYMBOL(sata_remove_phy);
+
+void sata_put_phy(struct sata_phy *phy)
+{
+ unsigned long flag;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ if (phy)
+ put_device(phy->dev);
+
+ spin_unlock_irqrestore(&phy_lock, flag);
+
+}
+EXPORT_SYMBOL(sata_put_phy);
diff --git a/drivers/ata/sata_phy.h b/drivers/ata/sata_phy.h
new file mode 100644
index 0000000..dc38683
--- /dev/null
+++ b/drivers/ata/sata_phy.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA utility framework definitions.
+ *
+ * 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.
+*/
+
+enum sata_phy_type {
+ SATA_PHY_GENERATION1,
+ SATA_PHY_GENERATION2,
+ SATA_PHY_GENERATION3,
+};
+
+struct sata_phy {
+ int (*init) (struct sata_phy *);
+ int (*shutdown) (struct sata_phy *);
+ struct device *dev;
+ void *priv_data;
+ enum sata_phy_type type;
+ struct list_head head;
+};
+
+static inline int sata_init_phy(struct sata_phy *x)
+{
+ if (x && x->init)
+ return x->init(x);
+
+ return -EINVAL;
+}
+
+static inline void sata_shutdown_phy(struct sata_phy *x)
+{
+ if (x && x->shutdown)
+ x->shutdown(x);
+}
+
+struct sata_phy *sata_get_phy(enum sata_phy_type);
+int sata_add_phy(struct sata_phy *, enum sata_phy_type);
+void sata_remove_phy(struct sata_phy *);
+void sata_put_phy(struct sata_phy *);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 3/6] DRIVERS: ATA: SATA PHY utility framework
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds SATA PHY utility framework APIs. The framework acts as an
interface between the SATA device and the PHY device. The SATA PHY device
registers itself with the framework through the APIs provided and the SATA
device finds and requests for an appropriate PHY device.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
drivers/ata/Kconfig | 9 ++++
drivers/ata/Makefile | 1 +
drivers/ata/sata_phy.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/sata_phy.h | 44 +++++++++++++++++++++
4 files changed, 153 insertions(+), 0 deletions(-)
create mode 100644 drivers/ata/sata_phy.c
create mode 100644 drivers/ata/sata_phy.h
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 27cecd3..0344b78 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -83,6 +83,15 @@ config SATA_AHCI_PLATFORM
If unsure, say N.
+config SATA_EXYNOS
+ bool "Exynos SATA AHCI support"
+ depends on I2C_S3C2410
+ help
+ This option enables support for Exynos AHCI Serial ATA
+ controllers.
+
+ If unsure, say N.
+
config SATA_FSL
tristate "Freescale 3.0Gbps SATA support"
depends on FSL_SOC
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index a454a13..bf3fd91 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
+obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/sata_phy.c b/drivers/ata/sata_phy.c
new file mode 100644
index 0000000..dbb4aa3
--- /dev/null
+++ b/drivers/ata/sata_phy.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA utility framework.
+ *
+ * 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/kernel.h>
+#include <linux/export.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include "sata_phy.h"
+
+static LIST_HEAD(phy_list);
+static DEFINE_SPINLOCK(phy_lock);
+
+struct sata_phy *sata_get_phy(enum sata_phy_type type)
+{
+ struct sata_phy *x = NULL;
+ unsigned long flag;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ list_for_each_entry(x, &phy_list, head) {
+ if (x->type == type) {
+ get_device(x->dev);
+ break;
+ }
+ }
+
+ spin_unlock_irqrestore(&phy_lock, flag);
+ return x;
+}
+EXPORT_SYMBOL(sata_get_phy);
+
+int sata_add_phy(struct sata_phy *phy, enum sata_phy_type type)
+{
+ unsigned long flag;
+ unsigned int ret = -EINVAL;
+ struct sata_phy *x;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ if (phy) {
+
+ list_for_each_entry(x, &phy_list, head) {
+ if (x->type == type) {
+ dev_err(phy->dev, "transceiver type already exists\n");
+ goto out;
+ }
+ }
+ phy->type = type;
+ list_add_tail(&phy->head, &phy_list);
+ ret = 0;
+ }
+
+ out:
+ spin_unlock_irqrestore(&phy_lock, flag);
+ return ret;
+}
+EXPORT_SYMBOL(sata_add_phy);
+
+void sata_remove_phy(struct sata_phy *phy)
+{
+ unsigned long flag;
+ struct sata_phy *x;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ if (phy) {
+ list_for_each_entry(x, &phy_list, head) {
+ if (x->type == phy->type)
+ list_del(&phy->head);
+ }
+ }
+
+ spin_unlock_irqrestore(&phy_lock, flag);
+}
+EXPORT_SYMBOL(sata_remove_phy);
+
+void sata_put_phy(struct sata_phy *phy)
+{
+ unsigned long flag;
+
+ spin_lock_irqsave(&phy_lock, flag);
+
+ if (phy)
+ put_device(phy->dev);
+
+ spin_unlock_irqrestore(&phy_lock, flag);
+
+}
+EXPORT_SYMBOL(sata_put_phy);
diff --git a/drivers/ata/sata_phy.h b/drivers/ata/sata_phy.h
new file mode 100644
index 0000000..dc38683
--- /dev/null
+++ b/drivers/ata/sata_phy.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA utility framework definitions.
+ *
+ * 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.
+*/
+
+enum sata_phy_type {
+ SATA_PHY_GENERATION1,
+ SATA_PHY_GENERATION2,
+ SATA_PHY_GENERATION3,
+};
+
+struct sata_phy {
+ int (*init) (struct sata_phy *);
+ int (*shutdown) (struct sata_phy *);
+ struct device *dev;
+ void *priv_data;
+ enum sata_phy_type type;
+ struct list_head head;
+};
+
+static inline int sata_init_phy(struct sata_phy *x)
+{
+ if (x && x->init)
+ return x->init(x);
+
+ return -EINVAL;
+}
+
+static inline void sata_shutdown_phy(struct sata_phy *x)
+{
+ if (x && x->shutdown)
+ x->shutdown(x);
+}
+
+struct sata_phy *sata_get_phy(enum sata_phy_type);
+int sata_add_phy(struct sata_phy *, enum sata_phy_type);
+void sata_remove_phy(struct sata_phy *);
+void sata_put_phy(struct sata_phy *);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 3/6] DRIVERS: ATA: SATA PHY utility framework
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-12 22:30 ` Tomasz Figa
-1 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:30 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi Vasanth,
On Tuesday 09 of October 2012 17:18:49 Vasanth Ananthan wrote:
> This patch adds SATA PHY utility framework APIs. The framework acts as
> an interface between the SATA device and the PHY device. The SATA PHY
> device registers itself with the framework through the APIs provided
> and the SATA device finds and requests for an appropriate PHY device.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/ata/Kconfig | 9 ++++
> drivers/ata/Makefile | 1 +
> drivers/ata/sata_phy.c | 99
> ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/sata_phy.h
> | 44 +++++++++++++++++++++
> 4 files changed, 153 insertions(+), 0 deletions(-)
> create mode 100644 drivers/ata/sata_phy.c
> create mode 100644 drivers/ata/sata_phy.h
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 27cecd3..0344b78 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -83,6 +83,15 @@ config SATA_AHCI_PLATFORM
>
> If unsure, say N.
>
> +config SATA_EXYNOS
> + bool "Exynos SATA AHCI support"
> + depends on I2C_S3C2410
> + help
> + This option enables support for Exynos AHCI Serial ATA
> + controllers.
> +
> + If unsure, say N.
> +
> config SATA_FSL
> tristate "Freescale 3.0Gbps SATA support"
> depends on FSL_SOC
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index a454a13..bf3fd91 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
> +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
If the framework introduced by this patch is supposed to be generic, maybe
a new Kconfig entry should be created for it, like CONFIG_SATA_PHY, which
would be selected by any drivers using it?
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/sata_phy.c b/drivers/ata/sata_phy.c
> new file mode 100644
> index 0000000..dbb4aa3
> --- /dev/null
> +++ b/drivers/ata/sata_phy.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * EXYNOS - SATA utility framework.
> + *
> + * 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/kernel.h>
> +#include <linux/export.h>
> +#include <linux/err.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +#include "sata_phy.h"
> +
> +static LIST_HEAD(phy_list);
> +static DEFINE_SPINLOCK(phy_lock);
> +
> +struct sata_phy *sata_get_phy(enum sata_phy_type type)
> +{
> + struct sata_phy *x = NULL;
> + unsigned long flag;
> +
> + spin_lock_irqsave(&phy_lock, flag);
> +
> + list_for_each_entry(x, &phy_list, head) {
> + if (x->type == type) {
> + get_device(x->dev);
> + break;
> + }
> + }
> +
> + spin_unlock_irqrestore(&phy_lock, flag);
> + return x;
> +}
> +EXPORT_SYMBOL(sata_get_phy);
> +
> +int sata_add_phy(struct sata_phy *phy, enum sata_phy_type type)
> +{
> + unsigned long flag;
> + unsigned int ret = -EINVAL;
> + struct sata_phy *x;
If you need to handle the situation when phy is NULL here, then why not
to:
if (!phy)
return -EINVAL;
and then make the code below unconditional?
> +
> + spin_lock_irqsave(&phy_lock, flag);
> +
> + if (phy) {
> +
> + list_for_each_entry(x, &phy_list, head) {
> + if (x->type == type) {
> + dev_err(phy->dev, "transceiver type already
exists\n");
> + goto out;
> + }
> + }
> + phy->type = type;
> + list_add_tail(&phy->head, &phy_list);
> + ret = 0;
> + }
> +
> + out:
> + spin_unlock_irqrestore(&phy_lock, flag);
> + return ret;
> +}
> +EXPORT_SYMBOL(sata_add_phy);
> +
> +void sata_remove_phy(struct sata_phy *phy)
> +{
> + unsigned long flag;
> + struct sata_phy *x;
Same here.
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 3/6] DRIVERS: ATA: SATA PHY utility framework
@ 2012-10-12 22:30 ` Tomasz Figa
0 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vasanth,
On Tuesday 09 of October 2012 17:18:49 Vasanth Ananthan wrote:
> This patch adds SATA PHY utility framework APIs. The framework acts as
> an interface between the SATA device and the PHY device. The SATA PHY
> device registers itself with the framework through the APIs provided
> and the SATA device finds and requests for an appropriate PHY device.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/ata/Kconfig | 9 ++++
> drivers/ata/Makefile | 1 +
> drivers/ata/sata_phy.c | 99
> ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/sata_phy.h
> | 44 +++++++++++++++++++++
> 4 files changed, 153 insertions(+), 0 deletions(-)
> create mode 100644 drivers/ata/sata_phy.c
> create mode 100644 drivers/ata/sata_phy.h
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 27cecd3..0344b78 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -83,6 +83,15 @@ config SATA_AHCI_PLATFORM
>
> If unsure, say N.
>
> +config SATA_EXYNOS
> + bool "Exynos SATA AHCI support"
> + depends on I2C_S3C2410
> + help
> + This option enables support for Exynos AHCI Serial ATA
> + controllers.
> +
> + If unsure, say N.
> +
> config SATA_FSL
> tristate "Freescale 3.0Gbps SATA support"
> depends on FSL_SOC
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index a454a13..bf3fd91 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
> +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
If the framework introduced by this patch is supposed to be generic, maybe
a new Kconfig entry should be created for it, like CONFIG_SATA_PHY, which
would be selected by any drivers using it?
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/sata_phy.c b/drivers/ata/sata_phy.c
> new file mode 100644
> index 0000000..dbb4aa3
> --- /dev/null
> +++ b/drivers/ata/sata_phy.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * EXYNOS - SATA utility framework.
> + *
> + * 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/kernel.h>
> +#include <linux/export.h>
> +#include <linux/err.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +#include "sata_phy.h"
> +
> +static LIST_HEAD(phy_list);
> +static DEFINE_SPINLOCK(phy_lock);
> +
> +struct sata_phy *sata_get_phy(enum sata_phy_type type)
> +{
> + struct sata_phy *x = NULL;
> + unsigned long flag;
> +
> + spin_lock_irqsave(&phy_lock, flag);
> +
> + list_for_each_entry(x, &phy_list, head) {
> + if (x->type == type) {
> + get_device(x->dev);
> + break;
> + }
> + }
> +
> + spin_unlock_irqrestore(&phy_lock, flag);
> + return x;
> +}
> +EXPORT_SYMBOL(sata_get_phy);
> +
> +int sata_add_phy(struct sata_phy *phy, enum sata_phy_type type)
> +{
> + unsigned long flag;
> + unsigned int ret = -EINVAL;
> + struct sata_phy *x;
If you need to handle the situation when phy is NULL here, then why not
to:
if (!phy)
return -EINVAL;
and then make the code below unconditional?
> +
> + spin_lock_irqsave(&phy_lock, flag);
> +
> + if (phy) {
> +
> + list_for_each_entry(x, &phy_list, head) {
> + if (x->type == type) {
> + dev_err(phy->dev, "transceiver type already
exists\n");
> + goto out;
> + }
> + }
> + phy->type = type;
> + list_add_tail(&phy->head, &phy_list);
> + ret = 0;
> + }
> +
> + out:
> + spin_unlock_irqrestore(&phy_lock, flag);
> + return ret;
> +}
> +EXPORT_SYMBOL(sata_add_phy);
> +
> +void sata_remove_phy(struct sata_phy *phy)
> +{
> + unsigned long flag;
> + struct sata_phy *x;
Same here.
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/6] DRIVERS: ATA: SATA PHY utility framework
2012-10-12 22:30 ` Tomasz Figa
@ 2012-10-15 9:46 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-15 9:46 UTC (permalink / raw)
To: Tomasz Figa
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi Tomasz,
On Sat, Oct 13, 2012 at 4:00 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vasanth,
>
> On Tuesday 09 of October 2012 17:18:49 Vasanth Ananthan wrote:
>> This patch adds SATA PHY utility framework APIs. The framework acts as
>> an interface between the SATA device and the PHY device. The SATA PHY
>> device registers itself with the framework through the APIs provided
>> and the SATA device finds and requests for an appropriate PHY device.
>>
>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>> ---
>> drivers/ata/Kconfig | 9 ++++
>> drivers/ata/Makefile | 1 +
>> drivers/ata/sata_phy.c | 99
>> ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/sata_phy.h
>> | 44 +++++++++++++++++++++
>> 4 files changed, 153 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/ata/sata_phy.c
>> create mode 100644 drivers/ata/sata_phy.h
>>
>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>> index 27cecd3..0344b78 100644
>> --- a/drivers/ata/Kconfig
>> +++ b/drivers/ata/Kconfig
>> @@ -83,6 +83,15 @@ config SATA_AHCI_PLATFORM
>>
>> If unsure, say N.
>>
>> +config SATA_EXYNOS
>> + bool "Exynos SATA AHCI support"
>> + depends on I2C_S3C2410
>> + help
>> + This option enables support for Exynos AHCI Serial ATA
>> + controllers.
>> +
>> + If unsure, say N.
>> +
>> config SATA_FSL
>> tristate "Freescale 3.0Gbps SATA support"
>> depends on FSL_SOC
>> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
>> index a454a13..bf3fd91 100644
>> --- a/drivers/ata/Makefile
>> +++ b/drivers/ata/Makefile
>> @@ -9,6 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
>> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
>> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
>> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
>> +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
>
>
> If the framework introduced by this patch is supposed to be generic, maybe
> a new Kconfig entry should be created for it, like CONFIG_SATA_PHY, which
> would be selected by any drivers using it?
>
>> +int sata_add_phy(struct sata_phy *phy, enum sata_phy_type type)
>> +{
>> + unsigned long flag;
>> + unsigned int ret = -EINVAL;
>> + struct sata_phy *x;
>
> If you need to handle the situation when phy is NULL here, then why not
> to:
>
> if (!phy)
> return -EINVAL;
>
> and then make the code below unconditional?
>
I'll incorporate the suggestions.
--
Vasanth
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 3/6] DRIVERS: ATA: SATA PHY utility framework
@ 2012-10-15 9:46 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-15 9:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Sat, Oct 13, 2012 at 4:00 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vasanth,
>
> On Tuesday 09 of October 2012 17:18:49 Vasanth Ananthan wrote:
>> This patch adds SATA PHY utility framework APIs. The framework acts as
>> an interface between the SATA device and the PHY device. The SATA PHY
>> device registers itself with the framework through the APIs provided
>> and the SATA device finds and requests for an appropriate PHY device.
>>
>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>> ---
>> drivers/ata/Kconfig | 9 ++++
>> drivers/ata/Makefile | 1 +
>> drivers/ata/sata_phy.c | 99
>> ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/sata_phy.h
>> | 44 +++++++++++++++++++++
>> 4 files changed, 153 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/ata/sata_phy.c
>> create mode 100644 drivers/ata/sata_phy.h
>>
>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>> index 27cecd3..0344b78 100644
>> --- a/drivers/ata/Kconfig
>> +++ b/drivers/ata/Kconfig
>> @@ -83,6 +83,15 @@ config SATA_AHCI_PLATFORM
>>
>> If unsure, say N.
>>
>> +config SATA_EXYNOS
>> + bool "Exynos SATA AHCI support"
>> + depends on I2C_S3C2410
>> + help
>> + This option enables support for Exynos AHCI Serial ATA
>> + controllers.
>> +
>> + If unsure, say N.
>> +
>> config SATA_FSL
>> tristate "Freescale 3.0Gbps SATA support"
>> depends on FSL_SOC
>> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
>> index a454a13..bf3fd91 100644
>> --- a/drivers/ata/Makefile
>> +++ b/drivers/ata/Makefile
>> @@ -9,6 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
>> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
>> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
>> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
>> +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
>
>
> If the framework introduced by this patch is supposed to be generic, maybe
> a new Kconfig entry should be created for it, like CONFIG_SATA_PHY, which
> would be selected by any drivers using it?
>
>> +int sata_add_phy(struct sata_phy *phy, enum sata_phy_type type)
>> +{
>> + unsigned long flag;
>> + unsigned int ret = -EINVAL;
>> + struct sata_phy *x;
>
> If you need to handle the situation when phy is NULL here, then why not
> to:
>
> if (!phy)
> return -EINVAL;
>
> and then make the code below unconditional?
>
I'll incorporate the suggestions.
--
Vasanth
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/6] ARM: S3C2410: I2C driver polling mode support
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 11:48 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: kgene.kim, jgarzik, linux
Cc: linux-arm-kernel, linux-samsung-soc, linux-ide, thomas.abraham,
Vasanth Ananthan, Vasanth Ananthan
This patch adds polling mode support for i2c s3c-2410 driver.
The I2C_SATAPHY controller lacks an interrupt line but the s3c-2410 driver
is interrupt driven. Hence this support is required for functioning
of the I2C_SATAPHY controller.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
drivers/i2c/busses/i2c-s3c2410.c | 84 +++++++++++++++++++++++++++++---------
1 files changed, 65 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 5ae3b02..699b530 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -48,6 +48,7 @@
#define QUIRK_S3C2440 (1 << 0)
#define QUIRK_HDMIPHY (1 << 1)
#define QUIRK_NO_GPIO (1 << 2)
+#define QUIRK_SATAPHY (1 << 3)
/* i2c controller state */
enum s3c24xx_i2c_state {
@@ -102,10 +103,14 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
};
MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
+static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat);
+
#ifdef CONFIG_OF
static const struct of_device_id s3c24xx_i2c_match[] = {
{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
{ .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
+ { .compatible = "samsung,s3c2440-sataphy-i2c",
+ .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
{ .compatible = "samsung,s3c2440-hdmiphy-i2c",
.data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
{},
@@ -146,7 +151,8 @@ static inline void s3c24xx_i2c_master_complete(struct s3c24xx_i2c *i2c, int ret)
if (ret)
i2c->msg_idx = ret;
- wake_up(&i2c->wait);
+ if (!(i2c->quirks & QUIRK_SATAPHY))
+ wake_up(&i2c->wait);
}
static inline void s3c24xx_i2c_disable_ack(struct s3c24xx_i2c *i2c)
@@ -184,6 +190,21 @@ static inline void s3c24xx_i2c_enable_irq(struct s3c24xx_i2c *i2c)
}
+static bool is_ack(struct s3c24xx_i2c *i2c)
+{
+ u32 time_out = i2c->tx_setup;
+ while (--time_out) {
+ if (readl(i2c->regs + S3C2410_IICCON)
+ & S3C2410_IICCON_IRQPEND) {
+ if (!(readl(i2c->regs + S3C2410_IICSTAT)
+ & S3C2410_IICSTAT_LASTBIT))
+ return true;
+ }
+ udelay(time_out);
+ }
+ return false;
+}
+
/* s3c24xx_i2c_message_start
*
* put the start of a message onto the bus
@@ -227,6 +248,21 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c,
stat |= S3C2410_IICSTAT_START;
writel(stat, i2c->regs + S3C2410_IICSTAT);
+
+ if (i2c->quirks & QUIRK_SATAPHY) {
+
+ while ((i2c->msg_num != 0) && is_ack(i2c)) {
+
+ i2c_s3c_irq_nextbyte(i2c, stat);
+
+ stat = readl(i2c->regs + S3C2410_IICSTAT);
+ if (stat & S3C2410_IICSTAT_ARBITR)
+ dev_err(i2c->dev, "deal with arbitration loss\n");
+
+ }
+
+ }
+
}
static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret)
@@ -552,22 +588,27 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
s3c24xx_i2c_message_start(i2c, msgs);
spin_unlock_irq(&i2c->lock);
- timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
-
ret = i2c->msg_idx;
- /* having these next two as dev_err() makes life very
- * noisy when doing an i2cdetect */
+ if (!(i2c->quirks & QUIRK_SATAPHY)) {
- if (timeout == 0)
- dev_dbg(i2c->dev, "timeout\n");
- else if (ret != num)
- dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret);
+ timeout = wait_event_timeout(i2c->wait,
+ i2c->msg_num == 0, HZ * 5);
- /* ensure the stop has been through the bus */
+ /* having these next two as dev_err() makes life very
+ * noisy when doing an i2cdetect */
+
+ if (timeout == 0)
+ dev_dbg(i2c->dev, "timeout\n");
+ else if (ret != num)
+ dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret);
+
+ /* ensure the stop has been through the bus */
+ }
dev_dbg(i2c->dev, "waiting for bus idle\n");
+
/* first, try busy waiting briefly */
do {
cpu_relax();
@@ -710,6 +751,9 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
if (div1 == 512)
iiccon |= S3C2410_IICCON_TXDIV_512;
+ if (i2c->quirks & QUIRK_SATAPHY)
+ iiccon |= S3C2410_IICCON_SCALE(2);
+
writel(iiccon, i2c->regs + S3C2410_IICCON);
if (i2c->quirks & QUIRK_S3C2440) {
@@ -1023,18 +1067,20 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
* ensure no current IRQs pending
*/
- i2c->irq = ret = platform_get_irq(pdev, 0);
- if (ret <= 0) {
- dev_err(&pdev->dev, "cannot find IRQ\n");
- goto err_iomap;
- }
+ if (!(i2c->quirks & QUIRK_SATAPHY)) {
+ i2c->irq = ret = platform_get_irq(pdev, 0);
+ if (ret <= 0) {
+ dev_err(&pdev->dev, "cannot find IRQ\n");
+ goto err_iomap;
+ }
- ret = request_irq(i2c->irq, s3c24xx_i2c_irq, 0,
+ ret = request_irq(i2c->irq, s3c24xx_i2c_irq, 0,
dev_name(&pdev->dev), i2c);
- if (ret != 0) {
- dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
- goto err_iomap;
+ if (ret != 0) {
+ dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
+ goto err_iomap;
+ }
}
ret = s3c24xx_i2c_register_cpufreq(i2c);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 4/6] ARM: S3C2410: I2C driver polling mode support
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds polling mode support for i2c s3c-2410 driver.
The I2C_SATAPHY controller lacks an interrupt line but the s3c-2410 driver
is interrupt driven. Hence this support is required for functioning
of the I2C_SATAPHY controller.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
drivers/i2c/busses/i2c-s3c2410.c | 84 +++++++++++++++++++++++++++++---------
1 files changed, 65 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 5ae3b02..699b530 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -48,6 +48,7 @@
#define QUIRK_S3C2440 (1 << 0)
#define QUIRK_HDMIPHY (1 << 1)
#define QUIRK_NO_GPIO (1 << 2)
+#define QUIRK_SATAPHY (1 << 3)
/* i2c controller state */
enum s3c24xx_i2c_state {
@@ -102,10 +103,14 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
};
MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
+static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat);
+
#ifdef CONFIG_OF
static const struct of_device_id s3c24xx_i2c_match[] = {
{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
{ .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
+ { .compatible = "samsung,s3c2440-sataphy-i2c",
+ .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
{ .compatible = "samsung,s3c2440-hdmiphy-i2c",
.data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
{},
@@ -146,7 +151,8 @@ static inline void s3c24xx_i2c_master_complete(struct s3c24xx_i2c *i2c, int ret)
if (ret)
i2c->msg_idx = ret;
- wake_up(&i2c->wait);
+ if (!(i2c->quirks & QUIRK_SATAPHY))
+ wake_up(&i2c->wait);
}
static inline void s3c24xx_i2c_disable_ack(struct s3c24xx_i2c *i2c)
@@ -184,6 +190,21 @@ static inline void s3c24xx_i2c_enable_irq(struct s3c24xx_i2c *i2c)
}
+static bool is_ack(struct s3c24xx_i2c *i2c)
+{
+ u32 time_out = i2c->tx_setup;
+ while (--time_out) {
+ if (readl(i2c->regs + S3C2410_IICCON)
+ & S3C2410_IICCON_IRQPEND) {
+ if (!(readl(i2c->regs + S3C2410_IICSTAT)
+ & S3C2410_IICSTAT_LASTBIT))
+ return true;
+ }
+ udelay(time_out);
+ }
+ return false;
+}
+
/* s3c24xx_i2c_message_start
*
* put the start of a message onto the bus
@@ -227,6 +248,21 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c,
stat |= S3C2410_IICSTAT_START;
writel(stat, i2c->regs + S3C2410_IICSTAT);
+
+ if (i2c->quirks & QUIRK_SATAPHY) {
+
+ while ((i2c->msg_num != 0) && is_ack(i2c)) {
+
+ i2c_s3c_irq_nextbyte(i2c, stat);
+
+ stat = readl(i2c->regs + S3C2410_IICSTAT);
+ if (stat & S3C2410_IICSTAT_ARBITR)
+ dev_err(i2c->dev, "deal with arbitration loss\n");
+
+ }
+
+ }
+
}
static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret)
@@ -552,22 +588,27 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
s3c24xx_i2c_message_start(i2c, msgs);
spin_unlock_irq(&i2c->lock);
- timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
-
ret = i2c->msg_idx;
- /* having these next two as dev_err() makes life very
- * noisy when doing an i2cdetect */
+ if (!(i2c->quirks & QUIRK_SATAPHY)) {
- if (timeout == 0)
- dev_dbg(i2c->dev, "timeout\n");
- else if (ret != num)
- dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret);
+ timeout = wait_event_timeout(i2c->wait,
+ i2c->msg_num == 0, HZ * 5);
- /* ensure the stop has been through the bus */
+ /* having these next two as dev_err() makes life very
+ * noisy when doing an i2cdetect */
+
+ if (timeout == 0)
+ dev_dbg(i2c->dev, "timeout\n");
+ else if (ret != num)
+ dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret);
+
+ /* ensure the stop has been through the bus */
+ }
dev_dbg(i2c->dev, "waiting for bus idle\n");
+
/* first, try busy waiting briefly */
do {
cpu_relax();
@@ -710,6 +751,9 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
if (div1 == 512)
iiccon |= S3C2410_IICCON_TXDIV_512;
+ if (i2c->quirks & QUIRK_SATAPHY)
+ iiccon |= S3C2410_IICCON_SCALE(2);
+
writel(iiccon, i2c->regs + S3C2410_IICCON);
if (i2c->quirks & QUIRK_S3C2440) {
@@ -1023,18 +1067,20 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
* ensure no current IRQs pending
*/
- i2c->irq = ret = platform_get_irq(pdev, 0);
- if (ret <= 0) {
- dev_err(&pdev->dev, "cannot find IRQ\n");
- goto err_iomap;
- }
+ if (!(i2c->quirks & QUIRK_SATAPHY)) {
+ i2c->irq = ret = platform_get_irq(pdev, 0);
+ if (ret <= 0) {
+ dev_err(&pdev->dev, "cannot find IRQ\n");
+ goto err_iomap;
+ }
- ret = request_irq(i2c->irq, s3c24xx_i2c_irq, 0,
+ ret = request_irq(i2c->irq, s3c24xx_i2c_irq, 0,
dev_name(&pdev->dev), i2c);
- if (ret != 0) {
- dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
- goto err_iomap;
+ if (ret != 0) {
+ dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
+ goto err_iomap;
+ }
}
ret = s3c24xx_i2c_register_cpufreq(i2c);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 4/6] ARM: S3C2410: I2C driver polling mode support
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-12 22:36 ` Tomasz Figa
-1 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:36 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi,
On Tuesday 09 of October 2012 17:18:50 Vasanth Ananthan wrote:
> This patch adds polling mode support for i2c s3c-2410 driver.
> The I2C_SATAPHY controller lacks an interrupt line but the s3c-2410
> driver is interrupt driven. Hence this support is required for
> functioning of the I2C_SATAPHY controller.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/i2c/busses/i2c-s3c2410.c | 84
> +++++++++++++++++++++++++++++--------- 1 files changed, 65
> insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> b/drivers/i2c/busses/i2c-s3c2410.c index 5ae3b02..699b530 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -48,6 +48,7 @@
> #define QUIRK_S3C2440 (1 << 0)
> #define QUIRK_HDMIPHY (1 << 1)
> #define QUIRK_NO_GPIO (1 << 2)
> +#define QUIRK_SATAPHY (1 << 3)
>
> /* i2c controller state */
> enum s3c24xx_i2c_state {
> @@ -102,10 +103,14 @@ static struct platform_device_id
> s3c24xx_driver_ids[] = { };
> MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
>
> +static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long
> iicstat); +
> #ifdef CONFIG_OF
> static const struct of_device_id s3c24xx_i2c_match[] = {
> { .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
> { .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440
> }, + { .compatible = "samsung,s3c2440-sataphy-i2c",
> + .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
nitpick: Please insert spaces around bitwise OR operators.
> { .compatible = "samsung,s3c2440-hdmiphy-i2c",
> .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
> {},
> @@ -146,7 +151,8 @@ static inline void
> s3c24xx_i2c_master_complete(struct s3c24xx_i2c *i2c, int ret) if (ret)
> i2c->msg_idx = ret;
>
> - wake_up(&i2c->wait);
> + if (!(i2c->quirks & QUIRK_SATAPHY))
> + wake_up(&i2c->wait);
> }
>
> static inline void s3c24xx_i2c_disable_ack(struct s3c24xx_i2c *i2c)
> @@ -184,6 +190,21 @@ static inline void s3c24xx_i2c_enable_irq(struct
> s3c24xx_i2c *i2c) }
>
>
> +static bool is_ack(struct s3c24xx_i2c *i2c)
> +{
> + u32 time_out = i2c->tx_setup;
nitpick: Separate local variable declaration from code with a blank line.
> + while (--time_out) {
> + if (readl(i2c->regs + S3C2410_IICCON)
> + & S3C2410_IICCON_IRQPEND) {
> + if (!(readl(i2c->regs + S3C2410_IICSTAT)
> + & S3C2410_IICSTAT_LASTBIT))
> + return true;
> + }
> + udelay(time_out);
> + }
nitpick: Here a blank line would be fine.
> + return false;
> +}
> +
> /* s3c24xx_i2c_message_start
> *
> * put the start of a message onto the bus
> @@ -227,6 +248,21 @@ static void s3c24xx_i2c_message_start(struct
> s3c24xx_i2c *i2c,
>
> stat |= S3C2410_IICSTAT_START;
> writel(stat, i2c->regs + S3C2410_IICSTAT);
> +
> + if (i2c->quirks & QUIRK_SATAPHY) {
> +
nitpick: Unnecessary blank line.
> + while ((i2c->msg_num != 0) && is_ack(i2c)) {
> +
Same here.
> + i2c_s3c_irq_nextbyte(i2c, stat);
> +
> + stat = readl(i2c->regs + S3C2410_IICSTAT);
> + if (stat & S3C2410_IICSTAT_ARBITR)
> + dev_err(i2c->dev, "deal with arbitration loss\n");
> +
Same here.
> + }
> +
Same here.
> + }
> +
Same here.
> }
>
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 4/6] ARM: S3C2410: I2C driver polling mode support
@ 2012-10-12 22:36 ` Tomasz Figa
0 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:36 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tuesday 09 of October 2012 17:18:50 Vasanth Ananthan wrote:
> This patch adds polling mode support for i2c s3c-2410 driver.
> The I2C_SATAPHY controller lacks an interrupt line but the s3c-2410
> driver is interrupt driven. Hence this support is required for
> functioning of the I2C_SATAPHY controller.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/i2c/busses/i2c-s3c2410.c | 84
> +++++++++++++++++++++++++++++--------- 1 files changed, 65
> insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> b/drivers/i2c/busses/i2c-s3c2410.c index 5ae3b02..699b530 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -48,6 +48,7 @@
> #define QUIRK_S3C2440 (1 << 0)
> #define QUIRK_HDMIPHY (1 << 1)
> #define QUIRK_NO_GPIO (1 << 2)
> +#define QUIRK_SATAPHY (1 << 3)
>
> /* i2c controller state */
> enum s3c24xx_i2c_state {
> @@ -102,10 +103,14 @@ static struct platform_device_id
> s3c24xx_driver_ids[] = { };
> MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
>
> +static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long
> iicstat); +
> #ifdef CONFIG_OF
> static const struct of_device_id s3c24xx_i2c_match[] = {
> { .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
> { .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440
> }, + { .compatible = "samsung,s3c2440-sataphy-i2c",
> + .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
nitpick: Please insert spaces around bitwise OR operators.
> { .compatible = "samsung,s3c2440-hdmiphy-i2c",
> .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
> {},
> @@ -146,7 +151,8 @@ static inline void
> s3c24xx_i2c_master_complete(struct s3c24xx_i2c *i2c, int ret) if (ret)
> i2c->msg_idx = ret;
>
> - wake_up(&i2c->wait);
> + if (!(i2c->quirks & QUIRK_SATAPHY))
> + wake_up(&i2c->wait);
> }
>
> static inline void s3c24xx_i2c_disable_ack(struct s3c24xx_i2c *i2c)
> @@ -184,6 +190,21 @@ static inline void s3c24xx_i2c_enable_irq(struct
> s3c24xx_i2c *i2c) }
>
>
> +static bool is_ack(struct s3c24xx_i2c *i2c)
> +{
> + u32 time_out = i2c->tx_setup;
nitpick: Separate local variable declaration from code with a blank line.
> + while (--time_out) {
> + if (readl(i2c->regs + S3C2410_IICCON)
> + & S3C2410_IICCON_IRQPEND) {
> + if (!(readl(i2c->regs + S3C2410_IICSTAT)
> + & S3C2410_IICSTAT_LASTBIT))
> + return true;
> + }
> + udelay(time_out);
> + }
nitpick: Here a blank line would be fine.
> + return false;
> +}
> +
> /* s3c24xx_i2c_message_start
> *
> * put the start of a message onto the bus
> @@ -227,6 +248,21 @@ static void s3c24xx_i2c_message_start(struct
> s3c24xx_i2c *i2c,
>
> stat |= S3C2410_IICSTAT_START;
> writel(stat, i2c->regs + S3C2410_IICSTAT);
> +
> + if (i2c->quirks & QUIRK_SATAPHY) {
> +
nitpick: Unnecessary blank line.
> + while ((i2c->msg_num != 0) && is_ack(i2c)) {
> +
Same here.
> + i2c_s3c_irq_nextbyte(i2c, stat);
> +
> + stat = readl(i2c->regs + S3C2410_IICSTAT);
> + if (stat & S3C2410_IICSTAT_ARBITR)
> + dev_err(i2c->dev, "deal with arbitration loss\n");
> +
Same here.
> + }
> +
Same here.
> + }
> +
Same here.
> }
>
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/6] ARM: S3C2410: I2C driver polling mode support
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-15 8:33 ` Heiko Stübner
-1 siblings, 0 replies; 42+ messages in thread
From: Heiko Stübner @ 2012-10-15 8:33 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Am Dienstag, 9. Oktober 2012, 13:48:50 schrieb Vasanth Ananthan:
> This patch adds polling mode support for i2c s3c-2410 driver.
> The I2C_SATAPHY controller lacks an interrupt line but the s3c-2410 driver
> is interrupt driven. Hence this support is required for functioning
> of the I2C_SATAPHY controller.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/i2c/busses/i2c-s3c2410.c | 84
> +++++++++++++++++++++++++++++--------- 1 files changed, 65 insertions(+),
> 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> b/drivers/i2c/busses/i2c-s3c2410.c index 5ae3b02..699b530 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
[...]
> @@ -102,10 +103,14 @@ static struct platform_device_id s3c24xx_driver_ids[]
> = { };
> MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
>
> +static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long
> iicstat); +
> #ifdef CONFIG_OF
> static const struct of_device_id s3c24xx_i2c_match[] = {
> { .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
> { .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
> + { .compatible = "samsung,s3c2440-sataphy-i2c",
> + .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
> { .compatible = "samsung,s3c2440-hdmiphy-i2c",
> .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
> {},
Out of curiosity, why is it called s3c2440-sataphy-i2c when it's an exynos
specific component?
Heiko
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 4/6] ARM: S3C2410: I2C driver polling mode support
@ 2012-10-15 8:33 ` Heiko Stübner
0 siblings, 0 replies; 42+ messages in thread
From: Heiko Stübner @ 2012-10-15 8:33 UTC (permalink / raw)
To: linux-arm-kernel
Am Dienstag, 9. Oktober 2012, 13:48:50 schrieb Vasanth Ananthan:
> This patch adds polling mode support for i2c s3c-2410 driver.
> The I2C_SATAPHY controller lacks an interrupt line but the s3c-2410 driver
> is interrupt driven. Hence this support is required for functioning
> of the I2C_SATAPHY controller.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/i2c/busses/i2c-s3c2410.c | 84
> +++++++++++++++++++++++++++++--------- 1 files changed, 65 insertions(+),
> 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> b/drivers/i2c/busses/i2c-s3c2410.c index 5ae3b02..699b530 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
[...]
> @@ -102,10 +103,14 @@ static struct platform_device_id s3c24xx_driver_ids[]
> = { };
> MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
>
> +static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long
> iicstat); +
> #ifdef CONFIG_OF
> static const struct of_device_id s3c24xx_i2c_match[] = {
> { .compatible = "samsung,s3c2410-i2c", .data = (void *)0 },
> { .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440 },
> + { .compatible = "samsung,s3c2440-sataphy-i2c",
> + .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
> { .compatible = "samsung,s3c2440-hdmiphy-i2c",
> .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
> {},
Out of curiosity, why is it called s3c2440-sataphy-i2c when it's an exynos
specific component?
Heiko
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/6] ARM: EYNOS5: SATA controller driver
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 11:48 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: kgene.kim, jgarzik, linux
Cc: linux-arm-kernel, linux-samsung-soc, linux-ide, thomas.abraham,
Vasanth Ananthan, Vasanth Ananthan
This patch adds a platform driver for SATA controller.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
arch/arm/mach-exynos/include/mach/regs-pmu.h | 3 +
drivers/ata/Makefile | 2 +-
drivers/ata/sata_exynos.c | 274 ++++++++++++++++++++++++++
3 files changed, 278 insertions(+), 1 deletions(-)
create mode 100644 drivers/ata/sata_exynos.c
diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h b/arch/arm/mach-exynos/include/mach/regs-pmu.h
index d4e392b..3716ce8 100644
--- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
+++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
@@ -367,4 +367,7 @@
#define EXYNOS5_OPTION_USE_RETENTION (1 << 4)
+/* Only for EXYNOS5250 */
+#define EXYNOS5_SATA_PHY_CONTROL S5P_PMUREG(0x0724)
+
#endif /* __ASM_ARCH_REGS_PMU_H */
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index bf3fd91..33fb2f6 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
-obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
+obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o libahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/sata_exynos.c b/drivers/ata/sata_exynos.c
new file mode 100644
index 0000000..877039e
--- /dev/null
+++ b/drivers/ata/sata_exynos.c
@@ -0,0 +1,274 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA controller driver
+ *
+ * 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/kernel.h>
+#include <linux/gfp.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/libata.h>
+#include <linux/ahci_platform.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+
+#include "ahci.h"
+#include "sata_phy.h"
+
+#define SATA_FREQ (66*1000*1000)
+
+static const struct ata_port_info ahci_port_info[] = {
+ [0] = {
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_ops,
+ },
+};
+
+static struct scsi_host_template ahci_platform_sht = {
+ AHCI_SHT("ahci_platform"),
+};
+
+struct exynos_sata {
+ struct clk *sclk;
+ struct clk *clk;
+ struct sata_phy *phy;
+ int irq;
+};
+
+static int __init exynos_sata_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ata_port_info pi = ahci_port_info[0];
+ const struct ata_port_info *ppi[] = { &pi, NULL };
+ struct ahci_host_priv *hpriv;
+ struct exynos_sata *sata;
+ struct ata_host *host;
+ struct resource *mem;
+ int n_ports, i, ret;
+
+ sata = kzalloc(sizeof(*sata), GFP_KERNEL);
+ if (!sata) {
+ dev_err(dev, "can't alloc sata\n");
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+ if (!hpriv) {
+ dev_err(dev, "can't alloc ahci_host_priv\n");
+ ret = -ENOMEM;
+ goto err1;
+ }
+
+ hpriv->flags |= (unsigned long)pi.private_data;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem) {
+ dev_err(dev, "no mmio space\n");
+ ret = -EINVAL;
+ goto err2;
+ }
+
+ sata->irq = platform_get_irq(pdev, 0);
+ if (sata->irq <= 0) {
+ dev_err(dev, "no irq\n");
+ ret = -EINVAL;
+ goto err2;
+ }
+
+ hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+ if (!hpriv->mmio) {
+ dev_err(dev, "can't map %pR\n", mem);
+ ret = -ENOMEM;
+ goto err2;
+ }
+
+ sata->sclk = clk_get(dev, "sclk_sata");
+ if (IS_ERR(sata->sclk)) {
+ dev_err(dev, "failed to get sclk_sata\n");
+ ret = PTR_ERR(sata->sclk);
+ goto err3;
+ }
+ clk_enable(sata->sclk);
+
+ clk_set_rate(sata->sclk, SATA_FREQ);
+
+ sata->clk = clk_get(dev, "sata");
+ if (IS_ERR(sata->clk)) {
+ dev_err(dev, "failed to get sata clock\n");
+ ret = PTR_ERR(sata->clk);
+ goto err4;
+ }
+ clk_enable(sata->clk);
+
+ /* Get a gen 3 PHY controller */
+
+ sata->phy = sata_get_phy(SATA_PHY_GENERATION3);
+ if (IS_ERR(sata->phy)) {
+ dev_err(dev, "failed to get sata phy\n");
+ ret = PTR_ERR(sata->clk);
+ goto err5;
+ }
+
+ /* Initialize the controller */
+
+ ret = sata_init_phy(sata->phy);
+ if (ret)
+ goto err6;
+
+ ahci_save_initial_config(dev, hpriv, 0, 0);
+
+ /* prepare host */
+ if (hpriv->cap & HOST_CAP_NCQ)
+ pi.flags |= ATA_FLAG_NCQ;
+
+ if (hpriv->cap & HOST_CAP_PMP)
+ pi.flags |= ATA_FLAG_PMP;
+
+ ahci_set_em_messages(hpriv, &pi);
+
+ /* CAP.NP sometimes indicate the index of the last enabled
+ * port, at other times, that of the last possible port, so
+ * determining the maximum port number requires looking at
+ * both CAP.NP and port_map.
+ */
+ n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
+
+ host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+ if (!host) {
+ ret = -ENOMEM;
+ goto err7;
+ }
+
+ host->private_data = hpriv;
+
+ if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
+ host->flags |= ATA_HOST_PARALLEL_SCAN;
+ else
+ pr_info(KERN_INFO
+ "ahci: SSS flag set, parallel bus scan disabled\n");
+
+ if (pi.flags & ATA_FLAG_EM)
+ ahci_reset_em(host);
+
+ for (i = 0; i < host->n_ports; i++) {
+ struct ata_port *ap = host->ports[i];
+
+ ata_port_desc(ap, "mmio %pR", mem);
+ ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
+
+ /* set enclosure management message type */
+ if (ap->flags & ATA_FLAG_EM)
+ ap->em_message_type = hpriv->em_msg_type;
+
+ /* disabled/not-implemented port */
+ if (!(hpriv->port_map & (1 << i)))
+ ap->ops = &ata_dummy_port_ops;
+ }
+
+ ret = ahci_reset_controller(host);
+ if (ret)
+ goto err7;
+
+ ahci_init_controller(host);
+ ahci_print_info(host, "platform");
+
+ ret = ata_host_activate(host, sata->irq, ahci_interrupt, IRQF_SHARED,
+ &ahci_platform_sht);
+ if (ret)
+ goto err7;
+
+ platform_set_drvdata(pdev, sata);
+
+ return 0;
+
+ err7:
+ sata_shutdown_phy(sata->phy);
+
+ err6:
+ sata_put_phy(sata->phy);
+
+ err5:
+ clk_disable(sata->clk);
+ clk_put(sata->clk);
+
+ err4:
+ clk_disable(sata->sclk);
+ clk_put(sata->sclk);
+
+ err3:
+ iounmap(hpriv->mmio);
+
+ err2:
+ kfree(hpriv);
+
+ err1:
+ kfree(sata);
+
+ return ret;
+}
+
+static int __devexit exynos_sata_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct exynos_sata *sata = platform_get_drvdata(pdev);
+ ata_host_detach(host);
+
+ sata_shutdown_phy(sata->phy);
+ sata_put_phy(sata->phy);
+
+ clk_disable(sata->sclk);
+ clk_put(sata->sclk);
+
+ clk_disable(sata->clk);
+ clk_put(sata->clk);
+
+ kfree(sata);
+
+ return 0;
+}
+
+static const struct of_device_id ahci_of_match[] = {
+ {.compatible = "samsung,exynos-sata-ahci",},
+};
+
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static struct platform_driver exynos_sata_driver = {
+ .remove = exynos_sata_remove,
+ .driver = {
+ .name = "ahci-sata",
+ .owner = THIS_MODULE,
+ .of_match_table = ahci_of_match,
+ },
+};
+
+static int __init exynos_sata_init(void)
+{
+ return platform_driver_probe(&exynos_sata_driver, exynos_sata_probe);
+}
+
+module_init(exynos_sata_init);
+
+static void __exit exynos_sata_exit(void)
+{
+ platform_driver_unregister(&exynos_sata_driver);
+}
+
+module_exit(exynos_sata_exit);
+
+MODULE_DESCRIPTION("EXYNOS SATA DRIVER");
+MODULE_AUTHOR("Vasanth Ananthan, <vasanth.a@samsung.com>");
+MODULE_LICENSE("GPL");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 5/6] ARM: EYNOS5: SATA controller driver
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds a platform driver for SATA controller.
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
arch/arm/mach-exynos/include/mach/regs-pmu.h | 3 +
drivers/ata/Makefile | 2 +-
drivers/ata/sata_exynos.c | 274 ++++++++++++++++++++++++++
3 files changed, 278 insertions(+), 1 deletions(-)
create mode 100644 drivers/ata/sata_exynos.c
diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h b/arch/arm/mach-exynos/include/mach/regs-pmu.h
index d4e392b..3716ce8 100644
--- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
+++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
@@ -367,4 +367,7 @@
#define EXYNOS5_OPTION_USE_RETENTION (1 << 4)
+/* Only for EXYNOS5250 */
+#define EXYNOS5_SATA_PHY_CONTROL S5P_PMUREG(0x0724)
+
#endif /* __ASM_ARCH_REGS_PMU_H */
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index bf3fd91..33fb2f6 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
-obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
+obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o libahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/sata_exynos.c b/drivers/ata/sata_exynos.c
new file mode 100644
index 0000000..877039e
--- /dev/null
+++ b/drivers/ata/sata_exynos.c
@@ -0,0 +1,274 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA controller driver
+ *
+ * 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/kernel.h>
+#include <linux/gfp.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/libata.h>
+#include <linux/ahci_platform.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+
+#include "ahci.h"
+#include "sata_phy.h"
+
+#define SATA_FREQ (66*1000*1000)
+
+static const struct ata_port_info ahci_port_info[] = {
+ [0] = {
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_ops,
+ },
+};
+
+static struct scsi_host_template ahci_platform_sht = {
+ AHCI_SHT("ahci_platform"),
+};
+
+struct exynos_sata {
+ struct clk *sclk;
+ struct clk *clk;
+ struct sata_phy *phy;
+ int irq;
+};
+
+static int __init exynos_sata_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ata_port_info pi = ahci_port_info[0];
+ const struct ata_port_info *ppi[] = { &pi, NULL };
+ struct ahci_host_priv *hpriv;
+ struct exynos_sata *sata;
+ struct ata_host *host;
+ struct resource *mem;
+ int n_ports, i, ret;
+
+ sata = kzalloc(sizeof(*sata), GFP_KERNEL);
+ if (!sata) {
+ dev_err(dev, "can't alloc sata\n");
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
+ if (!hpriv) {
+ dev_err(dev, "can't alloc ahci_host_priv\n");
+ ret = -ENOMEM;
+ goto err1;
+ }
+
+ hpriv->flags |= (unsigned long)pi.private_data;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem) {
+ dev_err(dev, "no mmio space\n");
+ ret = -EINVAL;
+ goto err2;
+ }
+
+ sata->irq = platform_get_irq(pdev, 0);
+ if (sata->irq <= 0) {
+ dev_err(dev, "no irq\n");
+ ret = -EINVAL;
+ goto err2;
+ }
+
+ hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+ if (!hpriv->mmio) {
+ dev_err(dev, "can't map %pR\n", mem);
+ ret = -ENOMEM;
+ goto err2;
+ }
+
+ sata->sclk = clk_get(dev, "sclk_sata");
+ if (IS_ERR(sata->sclk)) {
+ dev_err(dev, "failed to get sclk_sata\n");
+ ret = PTR_ERR(sata->sclk);
+ goto err3;
+ }
+ clk_enable(sata->sclk);
+
+ clk_set_rate(sata->sclk, SATA_FREQ);
+
+ sata->clk = clk_get(dev, "sata");
+ if (IS_ERR(sata->clk)) {
+ dev_err(dev, "failed to get sata clock\n");
+ ret = PTR_ERR(sata->clk);
+ goto err4;
+ }
+ clk_enable(sata->clk);
+
+ /* Get a gen 3 PHY controller */
+
+ sata->phy = sata_get_phy(SATA_PHY_GENERATION3);
+ if (IS_ERR(sata->phy)) {
+ dev_err(dev, "failed to get sata phy\n");
+ ret = PTR_ERR(sata->clk);
+ goto err5;
+ }
+
+ /* Initialize the controller */
+
+ ret = sata_init_phy(sata->phy);
+ if (ret)
+ goto err6;
+
+ ahci_save_initial_config(dev, hpriv, 0, 0);
+
+ /* prepare host */
+ if (hpriv->cap & HOST_CAP_NCQ)
+ pi.flags |= ATA_FLAG_NCQ;
+
+ if (hpriv->cap & HOST_CAP_PMP)
+ pi.flags |= ATA_FLAG_PMP;
+
+ ahci_set_em_messages(hpriv, &pi);
+
+ /* CAP.NP sometimes indicate the index of the last enabled
+ * port, at other times, that of the last possible port, so
+ * determining the maximum port number requires looking at
+ * both CAP.NP and port_map.
+ */
+ n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
+
+ host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+ if (!host) {
+ ret = -ENOMEM;
+ goto err7;
+ }
+
+ host->private_data = hpriv;
+
+ if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
+ host->flags |= ATA_HOST_PARALLEL_SCAN;
+ else
+ pr_info(KERN_INFO
+ "ahci: SSS flag set, parallel bus scan disabled\n");
+
+ if (pi.flags & ATA_FLAG_EM)
+ ahci_reset_em(host);
+
+ for (i = 0; i < host->n_ports; i++) {
+ struct ata_port *ap = host->ports[i];
+
+ ata_port_desc(ap, "mmio %pR", mem);
+ ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
+
+ /* set enclosure management message type */
+ if (ap->flags & ATA_FLAG_EM)
+ ap->em_message_type = hpriv->em_msg_type;
+
+ /* disabled/not-implemented port */
+ if (!(hpriv->port_map & (1 << i)))
+ ap->ops = &ata_dummy_port_ops;
+ }
+
+ ret = ahci_reset_controller(host);
+ if (ret)
+ goto err7;
+
+ ahci_init_controller(host);
+ ahci_print_info(host, "platform");
+
+ ret = ata_host_activate(host, sata->irq, ahci_interrupt, IRQF_SHARED,
+ &ahci_platform_sht);
+ if (ret)
+ goto err7;
+
+ platform_set_drvdata(pdev, sata);
+
+ return 0;
+
+ err7:
+ sata_shutdown_phy(sata->phy);
+
+ err6:
+ sata_put_phy(sata->phy);
+
+ err5:
+ clk_disable(sata->clk);
+ clk_put(sata->clk);
+
+ err4:
+ clk_disable(sata->sclk);
+ clk_put(sata->sclk);
+
+ err3:
+ iounmap(hpriv->mmio);
+
+ err2:
+ kfree(hpriv);
+
+ err1:
+ kfree(sata);
+
+ return ret;
+}
+
+static int __devexit exynos_sata_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct exynos_sata *sata = platform_get_drvdata(pdev);
+ ata_host_detach(host);
+
+ sata_shutdown_phy(sata->phy);
+ sata_put_phy(sata->phy);
+
+ clk_disable(sata->sclk);
+ clk_put(sata->sclk);
+
+ clk_disable(sata->clk);
+ clk_put(sata->clk);
+
+ kfree(sata);
+
+ return 0;
+}
+
+static const struct of_device_id ahci_of_match[] = {
+ {.compatible = "samsung,exynos-sata-ahci",},
+};
+
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static struct platform_driver exynos_sata_driver = {
+ .remove = exynos_sata_remove,
+ .driver = {
+ .name = "ahci-sata",
+ .owner = THIS_MODULE,
+ .of_match_table = ahci_of_match,
+ },
+};
+
+static int __init exynos_sata_init(void)
+{
+ return platform_driver_probe(&exynos_sata_driver, exynos_sata_probe);
+}
+
+module_init(exynos_sata_init);
+
+static void __exit exynos_sata_exit(void)
+{
+ platform_driver_unregister(&exynos_sata_driver);
+}
+
+module_exit(exynos_sata_exit);
+
+MODULE_DESCRIPTION("EXYNOS SATA DRIVER");
+MODULE_AUTHOR("Vasanth Ananthan, <vasanth.a@samsung.com>");
+MODULE_LICENSE("GPL");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 5/6] ARM: EYNOS5: SATA controller driver
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-12 22:45 ` Tomasz Figa
-1 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:45 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi,
On Tuesday 09 of October 2012 17:18:51 Vasanth Ananthan wrote:
> This patch adds a platform driver for SATA controller.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> arch/arm/mach-exynos/include/mach/regs-pmu.h | 3 +
> drivers/ata/Makefile | 2 +-
> drivers/ata/sata_exynos.c | 274
> ++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 1
> deletions(-)
> create mode 100644 drivers/ata/sata_exynos.c
>
> diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> b/arch/arm/mach-exynos/include/mach/regs-pmu.h index d4e392b..3716ce8
> 100644
> --- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> +++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
> @@ -367,4 +367,7 @@
>
> #define EXYNOS5_OPTION_USE_RETENTION (1 << 4)
>
> +/* Only for EXYNOS5250 */
> +#define EXYNOS5_SATA_PHY_CONTROL S5P_PMUREG(0x0724)
> +
> #endif /* __ASM_ARCH_REGS_PMU_H */
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index bf3fd91..33fb2f6 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
> -obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
> +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o
> libahci.o
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/sata_exynos.c b/drivers/ata/sata_exynos.c
> new file mode 100644
> index 0000000..877039e
> --- /dev/null
> +++ b/drivers/ata/sata_exynos.c
> @@ -0,0 +1,274 @@
> +/*
> + * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * EXYNOS - SATA controller driver
> + *
> + * 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/kernel.h>
> +#include <linux/gfp.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/libata.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +
> +#include "ahci.h"
> +#include "sata_phy.h"
> +
> +#define SATA_FREQ (66*1000*1000)
> +
> +static const struct ata_port_info ahci_port_info[] = {
> + [0] = {
> + .flags = AHCI_FLAG_COMMON,
> + .pio_mask = ATA_PIO4,
> + .udma_mask = ATA_UDMA6,
> + .port_ops = &ahci_ops,
> + },
nitpick: Indentation of closing bracket is incorrect.
> +};
> +
> +static struct scsi_host_template ahci_platform_sht = {
> + AHCI_SHT("ahci_platform"),
> +};
> +
> +struct exynos_sata {
> + struct clk *sclk;
> + struct clk *clk;
> + struct sata_phy *phy;
> + int irq;
> +};
> +
> +static int __init exynos_sata_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ata_port_info pi = ahci_port_info[0];
> + const struct ata_port_info *ppi[] = { &pi, NULL };
> + struct ahci_host_priv *hpriv;
> + struct exynos_sata *sata;
> + struct ata_host *host;
> + struct resource *mem;
> + int n_ports, i, ret;
> +
> + sata = kzalloc(sizeof(*sata), GFP_KERNEL);
Why this one isn't also allocated using devm_kzalloc?
> + if (!sata) {
> + dev_err(dev, "can't alloc sata\n");
> + ret = -ENOMEM;
> + return ret;
return -ENOMEM;
> + }
> +
> + hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
> + if (!hpriv) {
> + dev_err(dev, "can't alloc ahci_host_priv\n");
> + ret = -ENOMEM;
> + goto err1;
> + }
> +
> + hpriv->flags |= (unsigned long)pi.private_data;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem) {
> + dev_err(dev, "no mmio space\n");
> + ret = -EINVAL;
> + goto err2;
> + }
> +
> + sata->irq = platform_get_irq(pdev, 0);
> + if (sata->irq <= 0) {
> + dev_err(dev, "no irq\n");
> + ret = -EINVAL;
> + goto err2;
> + }
> +
> + hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
> + if (!hpriv->mmio) {
> + dev_err(dev, "can't map %pR\n", mem);
> + ret = -ENOMEM;
> + goto err2;
> + }
> +
> + sata->sclk = clk_get(dev, "sclk_sata");
devm_clk_get?
> + if (IS_ERR(sata->sclk)) {
> + dev_err(dev, "failed to get sclk_sata\n");
> + ret = PTR_ERR(sata->sclk);
> + goto err3;
> + }
> + clk_enable(sata->sclk);
> +
> + clk_set_rate(sata->sclk, SATA_FREQ);
> +
> + sata->clk = clk_get(dev, "sata");
devm_clk_get?
> + if (IS_ERR(sata->clk)) {
> + dev_err(dev, "failed to get sata clock\n");
> + ret = PTR_ERR(sata->clk);
> + goto err4;
> + }
> + clk_enable(sata->clk);
> +
> + /* Get a gen 3 PHY controller */
> +
> + sata->phy = sata_get_phy(SATA_PHY_GENERATION3);
> + if (IS_ERR(sata->phy)) {
> + dev_err(dev, "failed to get sata phy\n");
> + ret = PTR_ERR(sata->clk);
> + goto err5;
> + }
> +
> + /* Initialize the controller */
> +
> + ret = sata_init_phy(sata->phy);
> + if (ret)
> + goto err6;
> +
> + ahci_save_initial_config(dev, hpriv, 0, 0);
> +
> + /* prepare host */
> + if (hpriv->cap & HOST_CAP_NCQ)
> + pi.flags |= ATA_FLAG_NCQ;
> +
> + if (hpriv->cap & HOST_CAP_PMP)
> + pi.flags |= ATA_FLAG_PMP;
> +
> + ahci_set_em_messages(hpriv, &pi);
> +
> + /* CAP.NP sometimes indicate the index of the last enabled
> + * port, at other times, that of the last possible port, so
> + * determining the maximum port number requires looking at
> + * both CAP.NP and port_map.
> + */
> + n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
> +
> + host = ata_host_alloc_pinfo(dev, ppi, n_ports);
> + if (!host) {
> + ret = -ENOMEM;
> + goto err7;
> + }
> +
> + host->private_data = hpriv;
> +
> + if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
> + host->flags |= ATA_HOST_PARALLEL_SCAN;
> + else
> + pr_info(KERN_INFO
> + "ahci: SSS flag set, parallel bus scan disabled\n");
> +
> + if (pi.flags & ATA_FLAG_EM)
> + ahci_reset_em(host);
> +
> + for (i = 0; i < host->n_ports; i++) {
> + struct ata_port *ap = host->ports[i];
> +
> + ata_port_desc(ap, "mmio %pR", mem);
> + ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
> +
> + /* set enclosure management message type */
> + if (ap->flags & ATA_FLAG_EM)
> + ap->em_message_type = hpriv->em_msg_type;
> +
> + /* disabled/not-implemented port */
> + if (!(hpriv->port_map & (1 << i)))
> + ap->ops = &ata_dummy_port_ops;
> + }
> +
> + ret = ahci_reset_controller(host);
> + if (ret)
> + goto err7;
> +
> + ahci_init_controller(host);
> + ahci_print_info(host, "platform");
> +
> + ret = ata_host_activate(host, sata->irq, ahci_interrupt,
IRQF_SHARED,
> + &ahci_platform_sht);
> + if (ret)
> + goto err7;
> +
> + platform_set_drvdata(pdev, sata);
> +
> + return 0;
> +
> + err7:
> + sata_shutdown_phy(sata->phy);
> +
> + err6:
> + sata_put_phy(sata->phy);
> +
> + err5:
> + clk_disable(sata->clk);
> + clk_put(sata->clk);
> +
> + err4:
> + clk_disable(sata->sclk);
> + clk_put(sata->sclk);
> +
> + err3:
> + iounmap(hpriv->mmio);
> +
> + err2:
> + kfree(hpriv);
> +
> + err1:
> + kfree(sata);
> +
> + return ret;
> +}
> +
> +static int __devexit exynos_sata_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ata_host *host = dev_get_drvdata(dev);
> + struct exynos_sata *sata = platform_get_drvdata(pdev);
> + ata_host_detach(host);
> +
> + sata_shutdown_phy(sata->phy);
> + sata_put_phy(sata->phy);
> +
> + clk_disable(sata->sclk);
> + clk_put(sata->sclk);
> +
> + clk_disable(sata->clk);
> + clk_put(sata->clk);
> +
> + kfree(sata);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ahci_of_match[] = {
> + {.compatible = "samsung,exynos-sata-ahci",},
nitpick: Space after opening and before closing bracket.
> +};
> +
> +MODULE_DEVICE_TABLE(of, ahci_of_match);
> +
> +static struct platform_driver exynos_sata_driver = {
> + .remove = exynos_sata_remove,
> + .driver = {
> + .name = "ahci-sata",
> + .owner = THIS_MODULE,
> + .of_match_table = ahci_of_match,
> + },
nitpick: Incorrect indentation.
> +};
> +
> +static int __init exynos_sata_init(void)
> +{
> + return platform_driver_probe(&exynos_sata_driver,
exynos_sata_probe);
> +}
> +
> +module_init(exynos_sata_init);
> +
> +static void __exit exynos_sata_exit(void)
> +{
> + platform_driver_unregister(&exynos_sata_driver);
> +}
> +
> +module_exit(exynos_sata_exit);
module_platform_driver(exynos_sata_driver);
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 5/6] ARM: EYNOS5: SATA controller driver
@ 2012-10-12 22:45 ` Tomasz Figa
0 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:45 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tuesday 09 of October 2012 17:18:51 Vasanth Ananthan wrote:
> This patch adds a platform driver for SATA controller.
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> arch/arm/mach-exynos/include/mach/regs-pmu.h | 3 +
> drivers/ata/Makefile | 2 +-
> drivers/ata/sata_exynos.c | 274
> ++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 1
> deletions(-)
> create mode 100644 drivers/ata/sata_exynos.c
>
> diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> b/arch/arm/mach-exynos/include/mach/regs-pmu.h index d4e392b..3716ce8
> 100644
> --- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> +++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
> @@ -367,4 +367,7 @@
>
> #define EXYNOS5_OPTION_USE_RETENTION (1 << 4)
>
> +/* Only for EXYNOS5250 */
> +#define EXYNOS5_SATA_PHY_CONTROL S5P_PMUREG(0x0724)
> +
> #endif /* __ASM_ARCH_REGS_PMU_H */
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index bf3fd91..33fb2f6 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
> -obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o libahci.o
> +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o
> libahci.o
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/sata_exynos.c b/drivers/ata/sata_exynos.c
> new file mode 100644
> index 0000000..877039e
> --- /dev/null
> +++ b/drivers/ata/sata_exynos.c
> @@ -0,0 +1,274 @@
> +/*
> + * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * EXYNOS - SATA controller driver
> + *
> + * 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/kernel.h>
> +#include <linux/gfp.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/libata.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +
> +#include "ahci.h"
> +#include "sata_phy.h"
> +
> +#define SATA_FREQ (66*1000*1000)
> +
> +static const struct ata_port_info ahci_port_info[] = {
> + [0] = {
> + .flags = AHCI_FLAG_COMMON,
> + .pio_mask = ATA_PIO4,
> + .udma_mask = ATA_UDMA6,
> + .port_ops = &ahci_ops,
> + },
nitpick: Indentation of closing bracket is incorrect.
> +};
> +
> +static struct scsi_host_template ahci_platform_sht = {
> + AHCI_SHT("ahci_platform"),
> +};
> +
> +struct exynos_sata {
> + struct clk *sclk;
> + struct clk *clk;
> + struct sata_phy *phy;
> + int irq;
> +};
> +
> +static int __init exynos_sata_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ata_port_info pi = ahci_port_info[0];
> + const struct ata_port_info *ppi[] = { &pi, NULL };
> + struct ahci_host_priv *hpriv;
> + struct exynos_sata *sata;
> + struct ata_host *host;
> + struct resource *mem;
> + int n_ports, i, ret;
> +
> + sata = kzalloc(sizeof(*sata), GFP_KERNEL);
Why this one isn't also allocated using devm_kzalloc?
> + if (!sata) {
> + dev_err(dev, "can't alloc sata\n");
> + ret = -ENOMEM;
> + return ret;
return -ENOMEM;
> + }
> +
> + hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
> + if (!hpriv) {
> + dev_err(dev, "can't alloc ahci_host_priv\n");
> + ret = -ENOMEM;
> + goto err1;
> + }
> +
> + hpriv->flags |= (unsigned long)pi.private_data;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem) {
> + dev_err(dev, "no mmio space\n");
> + ret = -EINVAL;
> + goto err2;
> + }
> +
> + sata->irq = platform_get_irq(pdev, 0);
> + if (sata->irq <= 0) {
> + dev_err(dev, "no irq\n");
> + ret = -EINVAL;
> + goto err2;
> + }
> +
> + hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
> + if (!hpriv->mmio) {
> + dev_err(dev, "can't map %pR\n", mem);
> + ret = -ENOMEM;
> + goto err2;
> + }
> +
> + sata->sclk = clk_get(dev, "sclk_sata");
devm_clk_get?
> + if (IS_ERR(sata->sclk)) {
> + dev_err(dev, "failed to get sclk_sata\n");
> + ret = PTR_ERR(sata->sclk);
> + goto err3;
> + }
> + clk_enable(sata->sclk);
> +
> + clk_set_rate(sata->sclk, SATA_FREQ);
> +
> + sata->clk = clk_get(dev, "sata");
devm_clk_get?
> + if (IS_ERR(sata->clk)) {
> + dev_err(dev, "failed to get sata clock\n");
> + ret = PTR_ERR(sata->clk);
> + goto err4;
> + }
> + clk_enable(sata->clk);
> +
> + /* Get a gen 3 PHY controller */
> +
> + sata->phy = sata_get_phy(SATA_PHY_GENERATION3);
> + if (IS_ERR(sata->phy)) {
> + dev_err(dev, "failed to get sata phy\n");
> + ret = PTR_ERR(sata->clk);
> + goto err5;
> + }
> +
> + /* Initialize the controller */
> +
> + ret = sata_init_phy(sata->phy);
> + if (ret)
> + goto err6;
> +
> + ahci_save_initial_config(dev, hpriv, 0, 0);
> +
> + /* prepare host */
> + if (hpriv->cap & HOST_CAP_NCQ)
> + pi.flags |= ATA_FLAG_NCQ;
> +
> + if (hpriv->cap & HOST_CAP_PMP)
> + pi.flags |= ATA_FLAG_PMP;
> +
> + ahci_set_em_messages(hpriv, &pi);
> +
> + /* CAP.NP sometimes indicate the index of the last enabled
> + * port, at other times, that of the last possible port, so
> + * determining the maximum port number requires looking at
> + * both CAP.NP and port_map.
> + */
> + n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
> +
> + host = ata_host_alloc_pinfo(dev, ppi, n_ports);
> + if (!host) {
> + ret = -ENOMEM;
> + goto err7;
> + }
> +
> + host->private_data = hpriv;
> +
> + if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
> + host->flags |= ATA_HOST_PARALLEL_SCAN;
> + else
> + pr_info(KERN_INFO
> + "ahci: SSS flag set, parallel bus scan disabled\n");
> +
> + if (pi.flags & ATA_FLAG_EM)
> + ahci_reset_em(host);
> +
> + for (i = 0; i < host->n_ports; i++) {
> + struct ata_port *ap = host->ports[i];
> +
> + ata_port_desc(ap, "mmio %pR", mem);
> + ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
> +
> + /* set enclosure management message type */
> + if (ap->flags & ATA_FLAG_EM)
> + ap->em_message_type = hpriv->em_msg_type;
> +
> + /* disabled/not-implemented port */
> + if (!(hpriv->port_map & (1 << i)))
> + ap->ops = &ata_dummy_port_ops;
> + }
> +
> + ret = ahci_reset_controller(host);
> + if (ret)
> + goto err7;
> +
> + ahci_init_controller(host);
> + ahci_print_info(host, "platform");
> +
> + ret = ata_host_activate(host, sata->irq, ahci_interrupt,
IRQF_SHARED,
> + &ahci_platform_sht);
> + if (ret)
> + goto err7;
> +
> + platform_set_drvdata(pdev, sata);
> +
> + return 0;
> +
> + err7:
> + sata_shutdown_phy(sata->phy);
> +
> + err6:
> + sata_put_phy(sata->phy);
> +
> + err5:
> + clk_disable(sata->clk);
> + clk_put(sata->clk);
> +
> + err4:
> + clk_disable(sata->sclk);
> + clk_put(sata->sclk);
> +
> + err3:
> + iounmap(hpriv->mmio);
> +
> + err2:
> + kfree(hpriv);
> +
> + err1:
> + kfree(sata);
> +
> + return ret;
> +}
> +
> +static int __devexit exynos_sata_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ata_host *host = dev_get_drvdata(dev);
> + struct exynos_sata *sata = platform_get_drvdata(pdev);
> + ata_host_detach(host);
> +
> + sata_shutdown_phy(sata->phy);
> + sata_put_phy(sata->phy);
> +
> + clk_disable(sata->sclk);
> + clk_put(sata->sclk);
> +
> + clk_disable(sata->clk);
> + clk_put(sata->clk);
> +
> + kfree(sata);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ahci_of_match[] = {
> + {.compatible = "samsung,exynos-sata-ahci",},
nitpick: Space after opening and before closing bracket.
> +};
> +
> +MODULE_DEVICE_TABLE(of, ahci_of_match);
> +
> +static struct platform_driver exynos_sata_driver = {
> + .remove = exynos_sata_remove,
> + .driver = {
> + .name = "ahci-sata",
> + .owner = THIS_MODULE,
> + .of_match_table = ahci_of_match,
> + },
nitpick: Incorrect indentation.
> +};
> +
> +static int __init exynos_sata_init(void)
> +{
> + return platform_driver_probe(&exynos_sata_driver,
exynos_sata_probe);
> +}
> +
> +module_init(exynos_sata_init);
> +
> +static void __exit exynos_sata_exit(void)
> +{
> + platform_driver_unregister(&exynos_sata_driver);
> +}
> +
> +module_exit(exynos_sata_exit);
module_platform_driver(exynos_sata_driver);
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-09 11:48 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: kgene.kim, jgarzik, linux
Cc: linux-arm-kernel, linux-samsung-soc, linux-ide, thomas.abraham,
Vasanth Ananthan, Vasanth Ananthan
This patch adds a platform driver and I2C client driver for SATA PHY controller
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
drivers/ata/Makefile | 2 +-
drivers/ata/sata_exynos_phy.c | 303 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 304 insertions(+), 1 deletions(-)
create mode 100644 drivers/ata/sata_exynos_phy.c
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 33fb2f6..769d945 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
-obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o libahci.o
+obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos_phy.o sata_exynos.o libahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/sata_exynos_phy.c b/drivers/ata/sata_exynos_phy.c
new file mode 100644
index 0000000..44861de
--- /dev/null
+++ b/drivers/ata/sata_exynos_phy.c
@@ -0,0 +1,303 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA PHY controller driver
+ *
+ * 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/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/ahci_platform.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+
+#include <plat/cpu.h>
+
+#include <mach/irqs.h>
+#include <mach/map.h>
+#include <mach/regs-pmu.h>
+#include <mach/regs-sata.h>
+
+#include "sata_phy.h"
+
+#define SATA_TIME_LIMIT 1000
+
+static struct i2c_client *i2c_client;
+
+static struct i2c_driver sataphy_i2c_driver;
+
+struct exynos_sata_phy {
+ void __iomem *mmio;
+ struct resource *mem;
+ struct clk *clk;
+};
+
+static bool sata_is_reg(void __iomem *base, u32 reg, u32 checkbit, u32 Status)
+{
+ if ((readl(base + reg) & checkbit) == Status)
+ return true;
+ else
+ return false;
+}
+
+static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
+ u32 Status)
+{
+ u16 time_limit_cnt = 0;
+ while (!sata_is_reg(base, reg, checkbit, Status)) {
+ if (time_limit_cnt == SATA_TIME_LIMIT)
+ return false;
+ udelay(1000);
+ time_limit_cnt++;
+ }
+ return true;
+}
+
+int sataphy_init(struct sata_phy *phy)
+{
+ int ret;
+ u32 val;
+
+ /* Values to be written to enable 40 bits interface */
+ u8 buf[] = { 0x3A, 0x0B };
+
+ struct exynos_sata_phy *sata_phy;
+
+ sata_phy = (struct exynos_sata_phy *)phy->priv_data;
+
+ clk_enable(sata_phy->clk);
+
+ writel(S5P_PMU_SATA_PHY_CONTROL_EN, EXYNOS5_SATA_PHY_CONTROL);
+
+ val = 0;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= 0xFF;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= LINK_RESET;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= RESET_CMN_RST_N;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+ val &= ~PHCTRLM_REF_RATE;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+
+ /* High speed enable for Gen3 */
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+ val |= PHCTRLM_HIGH_SPEED;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_CTRL0);
+ val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_CTRL0);
+
+ writel(SATA_PHY_GENERATION3, sata_phy->mmio + EXYNOS5_SATA_MODE0);
+
+ ret = i2c_master_send(i2c_client, buf, sizeof(buf));
+ if (ret < 0)
+ return -EINVAL;
+
+ /* release cmu reset */
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val &= ~RESET_CMN_RST_N;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= RESET_CMN_RST_N;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ if (wait_for_reg_status(sata_phy->mmio, EXYNOS5_SATA_PHSATA_STATM,
+ PHSTATM_PLL_LOCKED, 1)) {
+ return 0;
+ }
+ return -EINVAL;
+}
+
+int sataphy_shutdown(struct sata_phy *phy)
+{
+
+ struct exynos_sata_phy *sata_phy;
+
+ sata_phy = (struct exynos_sata_phy *)phy->priv_data;
+
+ clk_disable(sata_phy->clk);
+
+ return 0;
+}
+
+static int sata_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *i2c_id)
+{
+ i2c_client = client;
+ return 0;
+}
+
+static int __init sata_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_sata_phy *sataphy;
+ struct sata_phy *phy;
+ struct resource *res;
+ int ret = 0;
+
+ phy = kzalloc(sizeof(struct sata_phy), GFP_KERNEL);
+ if (!phy) {
+ dev_err(&pdev->dev, "failed to allocate memory\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ sataphy = kzalloc(sizeof(struct exynos_sata_phy), GFP_KERNEL);
+ if (!sataphy) {
+ dev_err(&pdev->dev, "failed to allocate memory\n");
+ ret = -ENOMEM;
+ goto err0;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "Could not find IO resource\n");
+ ret = -EINVAL;
+ goto err1;
+ }
+
+ sataphy->mem = request_mem_region(res->start, resource_size(res),
+ pdev->name);
+ if (!sataphy->mem) {
+ dev_err(&pdev->dev, "Could not request IO resource\n");
+ ret = -EINVAL;
+ goto err1;
+ }
+
+ sataphy->mmio =
+ ioremap(res->start, resource_size(res));
+
+ if (!sataphy->mmio) {
+ dev_err(&pdev->dev, "failed to remap IO\n");
+ ret = -ENOMEM;
+ goto err2;
+ }
+
+ sataphy->clk = clk_get(&pdev->dev, "sata_phy");
+ if (IS_ERR(sataphy->clk)) {
+ dev_err(&pdev->dev, "failed to get clk for PHY\n");
+ ret = PTR_ERR(sataphy->clk);
+ sataphy->clk = NULL;
+ goto err3;
+ }
+
+ phy->init = sataphy_init;
+ phy->shutdown = sataphy_shutdown;
+ phy->priv_data = (void *)sataphy;
+ phy->dev = &pdev->dev;
+
+ ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
+ if (ret < 0)
+ goto err4;
+
+ ret = i2c_add_driver(&sataphy_i2c_driver);
+ if (ret < 0)
+ goto err5;
+
+ platform_set_drvdata(pdev, phy);
+
+ return ret;
+
+ err5:
+ sata_remove_phy(phy);
+
+ err4:
+ clk_put(sataphy->clk);
+
+ err3:
+ iounmap(sataphy->mmio);
+
+ err2:
+ release_resource(sataphy->mem);
+
+ err1:
+ kfree(sataphy);
+
+ err0:
+ kfree(phy);
+
+ out:
+ return ret;
+}
+
+static int sata_phy_remove(struct platform_device *pdev)
+{
+ struct sata_phy *phy;
+ struct exynos_sata_phy *sataphy;
+
+ phy = platform_get_drvdata(pdev);
+
+ sataphy = (struct exynos_sata_phy *)phy->priv_data;
+ sata_remove_phy(phy);
+
+ iounmap(sataphy->mmio);
+
+ release_resource(sataphy->mem);
+
+ clk_disable(sataphy->clk);
+ clk_put(sataphy->clk);
+
+ kfree(sataphy);
+ kfree(phy);
+
+ return 0;
+}
+
+static const struct of_device_id sata_phy_of_match[] = {
+ {.compatible = "samsung,exynos-sata-phy",},
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, sata_phy_of_match);
+
+static const struct i2c_device_id phy_i2c_device_match[] = {
+ {"sataphy", 0},
+};
+
+MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
+
+static struct platform_driver sata_phy_driver = {
+ .probe = sata_phy_probe,
+ .remove = sata_phy_remove,
+ .driver = {
+ .name = "sata-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = sata_phy_of_match,
+ },
+};
+
+static struct i2c_driver sataphy_i2c_driver = {
+ .driver = {
+ .name = "i2c-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = phy_i2c_device_match,
+ },
+ .probe = sata_i2c_probe,
+ .id_table = phy_i2c_device_match,
+};
+
+module_platform_driver(sata_phy_driver);
+
+MODULE_DESCRIPTION("EXYNOS SATA PHY DRIVER");
+MODULE_AUTHOR("Vasanth Ananthan, <vasanth.a@samsung.com>");
+MODULE_LICENSE("GPL");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
@ 2012-10-09 11:48 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-09 11:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds a platform driver and I2C client driver for SATA PHY controller
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
drivers/ata/Makefile | 2 +-
drivers/ata/sata_exynos_phy.c | 303 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 304 insertions(+), 1 deletions(-)
create mode 100644 drivers/ata/sata_exynos_phy.c
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 33fb2f6..769d945 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
-obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o libahci.o
+obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos_phy.o sata_exynos.o libahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/sata_exynos_phy.c b/drivers/ata/sata_exynos_phy.c
new file mode 100644
index 0000000..44861de
--- /dev/null
+++ b/drivers/ata/sata_exynos_phy.c
@@ -0,0 +1,303 @@
+/*
+ * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * EXYNOS - SATA PHY controller driver
+ *
+ * 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/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/ahci_platform.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+
+#include <plat/cpu.h>
+
+#include <mach/irqs.h>
+#include <mach/map.h>
+#include <mach/regs-pmu.h>
+#include <mach/regs-sata.h>
+
+#include "sata_phy.h"
+
+#define SATA_TIME_LIMIT 1000
+
+static struct i2c_client *i2c_client;
+
+static struct i2c_driver sataphy_i2c_driver;
+
+struct exynos_sata_phy {
+ void __iomem *mmio;
+ struct resource *mem;
+ struct clk *clk;
+};
+
+static bool sata_is_reg(void __iomem *base, u32 reg, u32 checkbit, u32 Status)
+{
+ if ((readl(base + reg) & checkbit) == Status)
+ return true;
+ else
+ return false;
+}
+
+static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
+ u32 Status)
+{
+ u16 time_limit_cnt = 0;
+ while (!sata_is_reg(base, reg, checkbit, Status)) {
+ if (time_limit_cnt == SATA_TIME_LIMIT)
+ return false;
+ udelay(1000);
+ time_limit_cnt++;
+ }
+ return true;
+}
+
+int sataphy_init(struct sata_phy *phy)
+{
+ int ret;
+ u32 val;
+
+ /* Values to be written to enable 40 bits interface */
+ u8 buf[] = { 0x3A, 0x0B };
+
+ struct exynos_sata_phy *sata_phy;
+
+ sata_phy = (struct exynos_sata_phy *)phy->priv_data;
+
+ clk_enable(sata_phy->clk);
+
+ writel(S5P_PMU_SATA_PHY_CONTROL_EN, EXYNOS5_SATA_PHY_CONTROL);
+
+ val = 0;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= 0xFF;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= LINK_RESET;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= RESET_CMN_RST_N;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+ val &= ~PHCTRLM_REF_RATE;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+
+ /* High speed enable for Gen3 */
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+ val |= PHCTRLM_HIGH_SPEED;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_CTRL0);
+ val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_CTRL0);
+
+ writel(SATA_PHY_GENERATION3, sata_phy->mmio + EXYNOS5_SATA_MODE0);
+
+ ret = i2c_master_send(i2c_client, buf, sizeof(buf));
+ if (ret < 0)
+ return -EINVAL;
+
+ /* release cmu reset */
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val &= ~RESET_CMN_RST_N;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
+ val |= RESET_CMN_RST_N;
+ writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
+
+ if (wait_for_reg_status(sata_phy->mmio, EXYNOS5_SATA_PHSATA_STATM,
+ PHSTATM_PLL_LOCKED, 1)) {
+ return 0;
+ }
+ return -EINVAL;
+}
+
+int sataphy_shutdown(struct sata_phy *phy)
+{
+
+ struct exynos_sata_phy *sata_phy;
+
+ sata_phy = (struct exynos_sata_phy *)phy->priv_data;
+
+ clk_disable(sata_phy->clk);
+
+ return 0;
+}
+
+static int sata_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *i2c_id)
+{
+ i2c_client = client;
+ return 0;
+}
+
+static int __init sata_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_sata_phy *sataphy;
+ struct sata_phy *phy;
+ struct resource *res;
+ int ret = 0;
+
+ phy = kzalloc(sizeof(struct sata_phy), GFP_KERNEL);
+ if (!phy) {
+ dev_err(&pdev->dev, "failed to allocate memory\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ sataphy = kzalloc(sizeof(struct exynos_sata_phy), GFP_KERNEL);
+ if (!sataphy) {
+ dev_err(&pdev->dev, "failed to allocate memory\n");
+ ret = -ENOMEM;
+ goto err0;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "Could not find IO resource\n");
+ ret = -EINVAL;
+ goto err1;
+ }
+
+ sataphy->mem = request_mem_region(res->start, resource_size(res),
+ pdev->name);
+ if (!sataphy->mem) {
+ dev_err(&pdev->dev, "Could not request IO resource\n");
+ ret = -EINVAL;
+ goto err1;
+ }
+
+ sataphy->mmio =
+ ioremap(res->start, resource_size(res));
+
+ if (!sataphy->mmio) {
+ dev_err(&pdev->dev, "failed to remap IO\n");
+ ret = -ENOMEM;
+ goto err2;
+ }
+
+ sataphy->clk = clk_get(&pdev->dev, "sata_phy");
+ if (IS_ERR(sataphy->clk)) {
+ dev_err(&pdev->dev, "failed to get clk for PHY\n");
+ ret = PTR_ERR(sataphy->clk);
+ sataphy->clk = NULL;
+ goto err3;
+ }
+
+ phy->init = sataphy_init;
+ phy->shutdown = sataphy_shutdown;
+ phy->priv_data = (void *)sataphy;
+ phy->dev = &pdev->dev;
+
+ ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
+ if (ret < 0)
+ goto err4;
+
+ ret = i2c_add_driver(&sataphy_i2c_driver);
+ if (ret < 0)
+ goto err5;
+
+ platform_set_drvdata(pdev, phy);
+
+ return ret;
+
+ err5:
+ sata_remove_phy(phy);
+
+ err4:
+ clk_put(sataphy->clk);
+
+ err3:
+ iounmap(sataphy->mmio);
+
+ err2:
+ release_resource(sataphy->mem);
+
+ err1:
+ kfree(sataphy);
+
+ err0:
+ kfree(phy);
+
+ out:
+ return ret;
+}
+
+static int sata_phy_remove(struct platform_device *pdev)
+{
+ struct sata_phy *phy;
+ struct exynos_sata_phy *sataphy;
+
+ phy = platform_get_drvdata(pdev);
+
+ sataphy = (struct exynos_sata_phy *)phy->priv_data;
+ sata_remove_phy(phy);
+
+ iounmap(sataphy->mmio);
+
+ release_resource(sataphy->mem);
+
+ clk_disable(sataphy->clk);
+ clk_put(sataphy->clk);
+
+ kfree(sataphy);
+ kfree(phy);
+
+ return 0;
+}
+
+static const struct of_device_id sata_phy_of_match[] = {
+ {.compatible = "samsung,exynos-sata-phy",},
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, sata_phy_of_match);
+
+static const struct i2c_device_id phy_i2c_device_match[] = {
+ {"sataphy", 0},
+};
+
+MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
+
+static struct platform_driver sata_phy_driver = {
+ .probe = sata_phy_probe,
+ .remove = sata_phy_remove,
+ .driver = {
+ .name = "sata-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = sata_phy_of_match,
+ },
+};
+
+static struct i2c_driver sataphy_i2c_driver = {
+ .driver = {
+ .name = "i2c-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = phy_i2c_device_match,
+ },
+ .probe = sata_i2c_probe,
+ .id_table = phy_i2c_device_match,
+};
+
+module_platform_driver(sata_phy_driver);
+
+MODULE_DESCRIPTION("EXYNOS SATA PHY DRIVER");
+MODULE_AUTHOR("Vasanth Ananthan, <vasanth.a@samsung.com>");
+MODULE_LICENSE("GPL");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
2012-10-09 11:48 ` Vasanth Ananthan
@ 2012-10-12 22:50 ` Tomasz Figa
-1 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:50 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi,
On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
> This patch adds a platform driver and I2C client driver for SATA PHY
> controller
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/ata/Makefile | 2 +-
> drivers/ata/sata_exynos_phy.c | 303
> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
> insertions(+), 1 deletions(-)
> create mode 100644 drivers/ata/sata_exynos_phy.c
>
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 33fb2f6..769d945 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
> -obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o
> libahci.o +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o
> sata_exynos_phy.o sata_exynos.o libahci.o
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/sata_exynos_phy.c
> b/drivers/ata/sata_exynos_phy.c new file mode 100644
> index 0000000..44861de
> --- /dev/null
> +++ b/drivers/ata/sata_exynos_phy.c
> @@ -0,0 +1,303 @@
> +/*
> + * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * EXYNOS - SATA PHY controller driver
> + *
> + * 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/module.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +
> +#include <plat/cpu.h>
> +
> +#include <mach/irqs.h>
> +#include <mach/map.h>
> +#include <mach/regs-pmu.h>
> +#include <mach/regs-sata.h>
> +
> +#include "sata_phy.h"
> +
> +#define SATA_TIME_LIMIT 1000
> +
> +static struct i2c_client *i2c_client;
> +
> +static struct i2c_driver sataphy_i2c_driver;
> +
> +struct exynos_sata_phy {
> + void __iomem *mmio;
> + struct resource *mem;
> + struct clk *clk;
> +};
> +
> +static bool sata_is_reg(void __iomem *base, u32 reg, u32 checkbit, u32
> Status) +{
Please use lowercase for variable names.
> + if ((readl(base + reg) & checkbit) == Status)
> + return true;
> + else
> + return false;
> +}
> +
> +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32
> checkbit, + u32 Status)
Same here.
> +{
> + u16 time_limit_cnt = 0;
> + while (!sata_is_reg(base, reg, checkbit, Status)) {
> + if (time_limit_cnt == SATA_TIME_LIMIT)
> + return false;
> + udelay(1000);
> + time_limit_cnt++;
> + }
> + return true;
> +}
> +
> +int sataphy_init(struct sata_phy *phy)
static int sataphy_init(struct sata_phy *phy) ?
> +{
> + int ret;
> + u32 val;
> +
> + /* Values to be written to enable 40 bits interface */
> + u8 buf[] = { 0x3A, 0x0B };
> +
> + struct exynos_sata_phy *sata_phy;
> +
> + sata_phy = (struct exynos_sata_phy *)phy->priv_data;
> +
> + clk_enable(sata_phy->clk);
> +
> + writel(S5P_PMU_SATA_PHY_CONTROL_EN, EXYNOS5_SATA_PHY_CONTROL);
> +
> + val = 0;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= 0xFF;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= LINK_RESET;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= RESET_CMN_RST_N;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> + val &= ~PHCTRLM_REF_RATE;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> +
> + /* High speed enable for Gen3 */
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> + val |= PHCTRLM_HIGH_SPEED;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_CTRL0);
> + val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_CTRL0);
> +
> + writel(SATA_PHY_GENERATION3, sata_phy->mmio + EXYNOS5_SATA_MODE0);
> +
> + ret = i2c_master_send(i2c_client, buf, sizeof(buf));
> + if (ret < 0)
> + return -EINVAL;
> +
> + /* release cmu reset */
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val &= ~RESET_CMN_RST_N;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= RESET_CMN_RST_N;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + if (wait_for_reg_status(sata_phy->mmio, EXYNOS5_SATA_PHSATA_STATM,
> + PHSTATM_PLL_LOCKED, 1)) {
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +int sataphy_shutdown(struct sata_phy *phy)
static int sataphy_shutdown(struct sata_phy *phy) ?
> +{
> +
> + struct exynos_sata_phy *sata_phy;
> +
> + sata_phy = (struct exynos_sata_phy *)phy->priv_data;
> +
> + clk_disable(sata_phy->clk);
> +
> + return 0;
> +}
> +
> +static int sata_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *i2c_id)
> +{
> + i2c_client = client;
> + return 0;
> +}
> +
> +static int __init sata_phy_probe(struct platform_device *pdev)
> +{
> + struct exynos_sata_phy *sataphy;
> + struct sata_phy *phy;
> + struct resource *res;
> + int ret = 0;
> +
> + phy = kzalloc(sizeof(struct sata_phy), GFP_KERNEL);
devm_
> + if (!phy) {
> + dev_err(&pdev->dev, "failed to allocate memory\n");
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + sataphy = kzalloc(sizeof(struct exynos_sata_phy), GFP_KERNEL);
devm_
> + if (!sataphy) {
> + dev_err(&pdev->dev, "failed to allocate memory\n");
> + ret = -ENOMEM;
> + goto err0;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "Could not find IO resource\n");
> + ret = -EINVAL;
> + goto err1;
> + }
> +
> + sataphy->mem = request_mem_region(res->start, resource_size(res),
> + pdev->name);
devm_
> + if (!sataphy->mem) {
> + dev_err(&pdev->dev, "Could not request IO resource\n");
> + ret = -EINVAL;
> + goto err1;
> + }
> +
> + sataphy->mmio =
> + ioremap(res->start, resource_size(res));
devm_
> +
nitpick: Unnecessary blank line.
> + if (!sataphy->mmio) {
> + dev_err(&pdev->dev, "failed to remap IO\n");
> + ret = -ENOMEM;
> + goto err2;
> + }
> +
> + sataphy->clk = clk_get(&pdev->dev, "sata_phy");
devm_
> + if (IS_ERR(sataphy->clk)) {
> + dev_err(&pdev->dev, "failed to get clk for PHY\n");
> + ret = PTR_ERR(sataphy->clk);
> + sataphy->clk = NULL;
> + goto err3;
> + }
> +
> + phy->init = sataphy_init;
> + phy->shutdown = sataphy_shutdown;
> + phy->priv_data = (void *)sataphy;
> + phy->dev = &pdev->dev;
> +
> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
> + if (ret < 0)
> + goto err4;
Do you have any warranties that phy callbacks won't get called before i2c
device probes and sets i2c_client?
> +
> + ret = i2c_add_driver(&sataphy_i2c_driver);
> + if (ret < 0)
> + goto err5;
> +
> + platform_set_drvdata(pdev, phy);
> +
> + return ret;
> +
> + err5:
> + sata_remove_phy(phy);
> +
> + err4:
> + clk_put(sataphy->clk);
> +
> + err3:
> + iounmap(sataphy->mmio);
> +
> + err2:
> + release_resource(sataphy->mem);
> +
> + err1:
> + kfree(sataphy);
> +
> + err0:
> + kfree(phy);
> +
> + out:
> + return ret;
> +}
> +
> +static int sata_phy_remove(struct platform_device *pdev)
> +{
> + struct sata_phy *phy;
> + struct exynos_sata_phy *sataphy;
> +
> + phy = platform_get_drvdata(pdev);
> +
> + sataphy = (struct exynos_sata_phy *)phy->priv_data;
> + sata_remove_phy(phy);
> +
> + iounmap(sataphy->mmio);
> +
> + release_resource(sataphy->mem);
> +
> + clk_disable(sataphy->clk);
> + clk_put(sataphy->clk);
> +
> + kfree(sataphy);
> + kfree(phy);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sata_phy_of_match[] = {
> + {.compatible = "samsung,exynos-sata-phy",},
nitpick: Space after opening and before closing brackets.
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, sata_phy_of_match);
> +
> +static const struct i2c_device_id phy_i2c_device_match[] = {
> + {"sataphy", 0},
Same here.
> +};
> +
> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
> +
> +static struct platform_driver sata_phy_driver = {
> + .probe = sata_phy_probe,
> + .remove = sata_phy_remove,
> + .driver = {
> + .name = "sata-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = sata_phy_of_match,
> + },
nitpick: Wrong indentation of closing bracket.
> +};
> +
> +static struct i2c_driver sataphy_i2c_driver = {
> + .driver = {
> + .name = "i2c-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = phy_i2c_device_match,
> + },
nitpick: Wrong indentation of closing bracket.
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
@ 2012-10-12 22:50 ` Tomasz Figa
0 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-12 22:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
> This patch adds a platform driver and I2C client driver for SATA PHY
> controller
>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
> drivers/ata/Makefile | 2 +-
> drivers/ata/sata_exynos_phy.c | 303
> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
> insertions(+), 1 deletions(-)
> create mode 100644 drivers/ata/sata_exynos_phy.c
>
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 33fb2f6..769d945 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -9,7 +9,7 @@ obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
> obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o
> -obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o sata_exynos.o
> libahci.o +obj-$(CONFIG_SATA_EXYNOS) += sata_phy.o
> sata_exynos_phy.o sata_exynos.o libahci.o
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/sata_exynos_phy.c
> b/drivers/ata/sata_exynos_phy.c new file mode 100644
> index 0000000..44861de
> --- /dev/null
> +++ b/drivers/ata/sata_exynos_phy.c
> @@ -0,0 +1,303 @@
> +/*
> + * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * EXYNOS - SATA PHY controller driver
> + *
> + * 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/module.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +
> +#include <plat/cpu.h>
> +
> +#include <mach/irqs.h>
> +#include <mach/map.h>
> +#include <mach/regs-pmu.h>
> +#include <mach/regs-sata.h>
> +
> +#include "sata_phy.h"
> +
> +#define SATA_TIME_LIMIT 1000
> +
> +static struct i2c_client *i2c_client;
> +
> +static struct i2c_driver sataphy_i2c_driver;
> +
> +struct exynos_sata_phy {
> + void __iomem *mmio;
> + struct resource *mem;
> + struct clk *clk;
> +};
> +
> +static bool sata_is_reg(void __iomem *base, u32 reg, u32 checkbit, u32
> Status) +{
Please use lowercase for variable names.
> + if ((readl(base + reg) & checkbit) == Status)
> + return true;
> + else
> + return false;
> +}
> +
> +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32
> checkbit, + u32 Status)
Same here.
> +{
> + u16 time_limit_cnt = 0;
> + while (!sata_is_reg(base, reg, checkbit, Status)) {
> + if (time_limit_cnt == SATA_TIME_LIMIT)
> + return false;
> + udelay(1000);
> + time_limit_cnt++;
> + }
> + return true;
> +}
> +
> +int sataphy_init(struct sata_phy *phy)
static int sataphy_init(struct sata_phy *phy) ?
> +{
> + int ret;
> + u32 val;
> +
> + /* Values to be written to enable 40 bits interface */
> + u8 buf[] = { 0x3A, 0x0B };
> +
> + struct exynos_sata_phy *sata_phy;
> +
> + sata_phy = (struct exynos_sata_phy *)phy->priv_data;
> +
> + clk_enable(sata_phy->clk);
> +
> + writel(S5P_PMU_SATA_PHY_CONTROL_EN, EXYNOS5_SATA_PHY_CONTROL);
> +
> + val = 0;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= 0xFF;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= LINK_RESET;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= RESET_CMN_RST_N;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> + val &= ~PHCTRLM_REF_RATE;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> +
> + /* High speed enable for Gen3 */
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> + val |= PHCTRLM_HIGH_SPEED;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_PHSATA_CTRLM);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_CTRL0);
> + val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_CTRL0);
> +
> + writel(SATA_PHY_GENERATION3, sata_phy->mmio + EXYNOS5_SATA_MODE0);
> +
> + ret = i2c_master_send(i2c_client, buf, sizeof(buf));
> + if (ret < 0)
> + return -EINVAL;
> +
> + /* release cmu reset */
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val &= ~RESET_CMN_RST_N;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + val = readl(sata_phy->mmio + EXYNOS5_SATA_RESET);
> + val |= RESET_CMN_RST_N;
> + writel(val, sata_phy->mmio + EXYNOS5_SATA_RESET);
> +
> + if (wait_for_reg_status(sata_phy->mmio, EXYNOS5_SATA_PHSATA_STATM,
> + PHSTATM_PLL_LOCKED, 1)) {
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +int sataphy_shutdown(struct sata_phy *phy)
static int sataphy_shutdown(struct sata_phy *phy) ?
> +{
> +
> + struct exynos_sata_phy *sata_phy;
> +
> + sata_phy = (struct exynos_sata_phy *)phy->priv_data;
> +
> + clk_disable(sata_phy->clk);
> +
> + return 0;
> +}
> +
> +static int sata_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *i2c_id)
> +{
> + i2c_client = client;
> + return 0;
> +}
> +
> +static int __init sata_phy_probe(struct platform_device *pdev)
> +{
> + struct exynos_sata_phy *sataphy;
> + struct sata_phy *phy;
> + struct resource *res;
> + int ret = 0;
> +
> + phy = kzalloc(sizeof(struct sata_phy), GFP_KERNEL);
devm_
> + if (!phy) {
> + dev_err(&pdev->dev, "failed to allocate memory\n");
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + sataphy = kzalloc(sizeof(struct exynos_sata_phy), GFP_KERNEL);
devm_
> + if (!sataphy) {
> + dev_err(&pdev->dev, "failed to allocate memory\n");
> + ret = -ENOMEM;
> + goto err0;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "Could not find IO resource\n");
> + ret = -EINVAL;
> + goto err1;
> + }
> +
> + sataphy->mem = request_mem_region(res->start, resource_size(res),
> + pdev->name);
devm_
> + if (!sataphy->mem) {
> + dev_err(&pdev->dev, "Could not request IO resource\n");
> + ret = -EINVAL;
> + goto err1;
> + }
> +
> + sataphy->mmio =
> + ioremap(res->start, resource_size(res));
devm_
> +
nitpick: Unnecessary blank line.
> + if (!sataphy->mmio) {
> + dev_err(&pdev->dev, "failed to remap IO\n");
> + ret = -ENOMEM;
> + goto err2;
> + }
> +
> + sataphy->clk = clk_get(&pdev->dev, "sata_phy");
devm_
> + if (IS_ERR(sataphy->clk)) {
> + dev_err(&pdev->dev, "failed to get clk for PHY\n");
> + ret = PTR_ERR(sataphy->clk);
> + sataphy->clk = NULL;
> + goto err3;
> + }
> +
> + phy->init = sataphy_init;
> + phy->shutdown = sataphy_shutdown;
> + phy->priv_data = (void *)sataphy;
> + phy->dev = &pdev->dev;
> +
> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
> + if (ret < 0)
> + goto err4;
Do you have any warranties that phy callbacks won't get called before i2c
device probes and sets i2c_client?
> +
> + ret = i2c_add_driver(&sataphy_i2c_driver);
> + if (ret < 0)
> + goto err5;
> +
> + platform_set_drvdata(pdev, phy);
> +
> + return ret;
> +
> + err5:
> + sata_remove_phy(phy);
> +
> + err4:
> + clk_put(sataphy->clk);
> +
> + err3:
> + iounmap(sataphy->mmio);
> +
> + err2:
> + release_resource(sataphy->mem);
> +
> + err1:
> + kfree(sataphy);
> +
> + err0:
> + kfree(phy);
> +
> + out:
> + return ret;
> +}
> +
> +static int sata_phy_remove(struct platform_device *pdev)
> +{
> + struct sata_phy *phy;
> + struct exynos_sata_phy *sataphy;
> +
> + phy = platform_get_drvdata(pdev);
> +
> + sataphy = (struct exynos_sata_phy *)phy->priv_data;
> + sata_remove_phy(phy);
> +
> + iounmap(sataphy->mmio);
> +
> + release_resource(sataphy->mem);
> +
> + clk_disable(sataphy->clk);
> + clk_put(sataphy->clk);
> +
> + kfree(sataphy);
> + kfree(phy);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sata_phy_of_match[] = {
> + {.compatible = "samsung,exynos-sata-phy",},
nitpick: Space after opening and before closing brackets.
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, sata_phy_of_match);
> +
> +static const struct i2c_device_id phy_i2c_device_match[] = {
> + {"sataphy", 0},
Same here.
> +};
> +
> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
> +
> +static struct platform_driver sata_phy_driver = {
> + .probe = sata_phy_probe,
> + .remove = sata_phy_remove,
> + .driver = {
> + .name = "sata-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = sata_phy_of_match,
> + },
nitpick: Wrong indentation of closing bracket.
> +};
> +
> +static struct i2c_driver sataphy_i2c_driver = {
> + .driver = {
> + .name = "i2c-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = phy_i2c_device_match,
> + },
nitpick: Wrong indentation of closing bracket.
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
2012-10-12 22:50 ` Tomasz Figa
@ 2012-10-16 6:33 ` Vasanth Ananthan
-1 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-16 6:33 UTC (permalink / raw)
To: Tomasz Figa
Cc: kgene.kim, jgarzik, linux, linux-arm-kernel, linux-samsung-soc,
linux-ide, thomas.abraham, Vasanth Ananthan
Hi Tomasz,
On Sat, Oct 13, 2012 at 4:20 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi,
>
> On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
>> This patch adds a platform driver and I2C client driver for SATA PHY
>> controller
>>
>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>> ---
>> drivers/ata/Makefile | 2 +-
>> drivers/ata/sata_exynos_phy.c | 303
>> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
>> insertions(+), 1 deletions(-)
>> create mode 100644 drivers/ata/sata_exynos_phy.c
>>
[ ... ]
>> +
>> + phy->init = sataphy_init;
>> + phy->shutdown = sataphy_shutdown;
>> + phy->priv_data = (void *)sataphy;
>> + phy->dev = &pdev->dev;
>> +
>> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
>> + if (ret < 0)
>> + goto err4;
>
> Do you have any warranties that phy callbacks won't get called before i2c
> device probes and sets i2c_client?
>
>> +
>> + ret = i2c_add_driver(&sataphy_i2c_driver);
>> + if (ret < 0)
>> + goto err5;
>> +
>> + platform_set_drvdata(pdev, phy);
I shall register my i2c client driver before assigning the SATA PHY
call backs. Would that guarantee the calling of phy callbacks after
the i2c device probes?
--
Vasanth Ananthan
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
@ 2012-10-16 6:33 ` Vasanth Ananthan
0 siblings, 0 replies; 42+ messages in thread
From: Vasanth Ananthan @ 2012-10-16 6:33 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On Sat, Oct 13, 2012 at 4:20 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi,
>
> On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
>> This patch adds a platform driver and I2C client driver for SATA PHY
>> controller
>>
>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>> ---
>> drivers/ata/Makefile | 2 +-
>> drivers/ata/sata_exynos_phy.c | 303
>> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
>> insertions(+), 1 deletions(-)
>> create mode 100644 drivers/ata/sata_exynos_phy.c
>>
[ ... ]
>> +
>> + phy->init = sataphy_init;
>> + phy->shutdown = sataphy_shutdown;
>> + phy->priv_data = (void *)sataphy;
>> + phy->dev = &pdev->dev;
>> +
>> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
>> + if (ret < 0)
>> + goto err4;
>
> Do you have any warranties that phy callbacks won't get called before i2c
> device probes and sets i2c_client?
>
>> +
>> + ret = i2c_add_driver(&sataphy_i2c_driver);
>> + if (ret < 0)
>> + goto err5;
>> +
>> + platform_set_drvdata(pdev, phy);
I shall register my i2c client driver before assigning the SATA PHY
call backs. Would that guarantee the calling of phy callbacks after
the i2c device probes?
--
Vasanth Ananthan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
2012-10-16 6:33 ` Vasanth Ananthan
@ 2012-10-16 10:02 ` Tomasz Figa
-1 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-16 10:02 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: linux-samsung-soc, linux, Vasanth Ananthan, Tomasz Figa,
linux-ide, kgene.kim, thomas.abraham, jgarzik, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1950 bytes --]
Hi Vasanth,
On Tuesday 16 of October 2012 12:03:47 Vasanth Ananthan wrote:
> Hi Tomasz,
>
> On Sat, Oct 13, 2012 at 4:20 AM, Tomasz Figa <tomasz.figa@gmail.com>
wrote:
> > Hi,
> >
> > On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
> >> This patch adds a platform driver and I2C client driver for SATA PHY
> >> controller
> >>
> >> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> >> ---
> >>
> >> drivers/ata/Makefile | 2 +-
> >> drivers/ata/sata_exynos_phy.c | 303
> >>
> >> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
> >> insertions(+), 1 deletions(-)
> >>
> >> create mode 100644 drivers/ata/sata_exynos_phy.c
>
> [ ... ]
>
> >> +
> >> + phy->init = sataphy_init;
> >> + phy->shutdown = sataphy_shutdown;
> >> + phy->priv_data = (void *)sataphy;
> >> + phy->dev = &pdev->dev;
> >> +
> >> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
> >> + if (ret < 0)
> >> + goto err4;
> >
> > Do you have any warranties that phy callbacks won't get called before
> > i2c device probes and sets i2c_client?
> >
> >> +
> >> + ret = i2c_add_driver(&sataphy_i2c_driver);
> >> + if (ret < 0)
> >> + goto err5;
> >> +
> >> + platform_set_drvdata(pdev, phy);
>
> I shall register my i2c client driver before assigning the SATA PHY
> call backs. Would that guarantee the calling of phy callbacks after
> the i2c device probes?
No. You might still not have the sata phy i2c controller registered at
that time and so the i2c driver will not be probed.
The only definitive solution for this would be to register both platform
and i2c drivers in module init (instead of using module_platform_driver)
and defer the probe of platform driver until i2c device gets registered.
You can defer the probe by returning -EPROBE_DEFER after checking that
i2c_client is still NULL.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
[-- Attachment #1.2: Type: text/html, Size: 11180 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
@ 2012-10-16 10:02 ` Tomasz Figa
0 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-16 10:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vasanth,
On Tuesday 16 of October 2012 12:03:47 Vasanth Ananthan wrote:
> Hi Tomasz,
>
> On Sat, Oct 13, 2012 at 4:20 AM, Tomasz Figa <tomasz.figa@gmail.com>
wrote:
> > Hi,
> >
> > On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
> >> This patch adds a platform driver and I2C client driver for SATA PHY
> >> controller
> >>
> >> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> >> ---
> >>
> >> drivers/ata/Makefile | 2 +-
> >> drivers/ata/sata_exynos_phy.c | 303
> >>
> >> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
> >> insertions(+), 1 deletions(-)
> >>
> >> create mode 100644 drivers/ata/sata_exynos_phy.c
>
> [ ... ]
>
> >> +
> >> + phy->init = sataphy_init;
> >> + phy->shutdown = sataphy_shutdown;
> >> + phy->priv_data = (void *)sataphy;
> >> + phy->dev = &pdev->dev;
> >> +
> >> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
> >> + if (ret < 0)
> >> + goto err4;
> >
> > Do you have any warranties that phy callbacks won't get called before
> > i2c device probes and sets i2c_client?
> >
> >> +
> >> + ret = i2c_add_driver(&sataphy_i2c_driver);
> >> + if (ret < 0)
> >> + goto err5;
> >> +
> >> + platform_set_drvdata(pdev, phy);
>
> I shall register my i2c client driver before assigning the SATA PHY
> call backs. Would that guarantee the calling of phy callbacks after
> the i2c device probes?
No. You might still not have the sata phy i2c controller registered at
that time and so the i2c driver will not be probed.
The only definitive solution for this would be to register both platform
and i2c drivers in module init (instead of using module_platform_driver)
and defer the probe of platform driver until i2c device gets registered.
You can defer the probe by returning -EPROBE_DEFER after checking that
i2c_client is still NULL.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121016/36672b49/attachment-0001.html>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
2012-10-16 6:33 ` Vasanth Ananthan
@ 2012-10-17 15:42 ` Tomasz Figa
-1 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-17 15:42 UTC (permalink / raw)
To: Vasanth Ananthan
Cc: linux-samsung-soc, linux, Vasanth Ananthan, Tomasz Figa,
linux-ide, kgene.kim, thomas.abraham, jgarzik, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1950 bytes --]
Hi Vasanth,
On Tuesday 16 of October 2012 12:03:47 Vasanth Ananthan wrote:
> Hi Tomasz,
>
> On Sat, Oct 13, 2012 at 4:20 AM, Tomasz Figa <tomasz.figa@gmail.com>
wrote:
> > Hi,
> >
> > On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
> >> This patch adds a platform driver and I2C client driver for SATA PHY
> >> controller
> >>
> >> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> >> ---
> >>
> >> drivers/ata/Makefile | 2 +-
> >> drivers/ata/sata_exynos_phy.c | 303
> >>
> >> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
> >> insertions(+), 1 deletions(-)
> >>
> >> create mode 100644 drivers/ata/sata_exynos_phy.c
>
> [ ... ]
>
> >> +
> >> + phy->init = sataphy_init;
> >> + phy->shutdown = sataphy_shutdown;
> >> + phy->priv_data = (void *)sataphy;
> >> + phy->dev = &pdev->dev;
> >> +
> >> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
> >> + if (ret < 0)
> >> + goto err4;
> >
> > Do you have any warranties that phy callbacks won't get called before
> > i2c device probes and sets i2c_client?
> >
> >> +
> >> + ret = i2c_add_driver(&sataphy_i2c_driver);
> >> + if (ret < 0)
> >> + goto err5;
> >> +
> >> + platform_set_drvdata(pdev, phy);
>
> I shall register my i2c client driver before assigning the SATA PHY
> call backs. Would that guarantee the calling of phy callbacks after
> the i2c device probes?
No. You might still not have the sata phy i2c controller registered at
that time and so the i2c driver will not be probed.
The only definitive solution for this would be to register both platform
and i2c drivers in module init (instead of using module_platform_driver)
and defer the probe of platform driver until i2c device gets registered.
You can defer the probe by returning -EPROBE_DEFER after checking that
i2c_client is still NULL.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
[-- Attachment #1.2: Type: text/html, Size: 11180 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 6/6] ARM: EXYNOS5: SATA PHY controller driver
@ 2012-10-17 15:42 ` Tomasz Figa
0 siblings, 0 replies; 42+ messages in thread
From: Tomasz Figa @ 2012-10-17 15:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vasanth,
On Tuesday 16 of October 2012 12:03:47 Vasanth Ananthan wrote:
> Hi Tomasz,
>
> On Sat, Oct 13, 2012 at 4:20 AM, Tomasz Figa <tomasz.figa@gmail.com>
wrote:
> > Hi,
> >
> > On Tuesday 09 of October 2012 17:18:52 Vasanth Ananthan wrote:
> >> This patch adds a platform driver and I2C client driver for SATA PHY
> >> controller
> >>
> >> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> >> ---
> >>
> >> drivers/ata/Makefile | 2 +-
> >> drivers/ata/sata_exynos_phy.c | 303
> >>
> >> +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304
> >> insertions(+), 1 deletions(-)
> >>
> >> create mode 100644 drivers/ata/sata_exynos_phy.c
>
> [ ... ]
>
> >> +
> >> + phy->init = sataphy_init;
> >> + phy->shutdown = sataphy_shutdown;
> >> + phy->priv_data = (void *)sataphy;
> >> + phy->dev = &pdev->dev;
> >> +
> >> + ret = sata_add_phy(phy, SATA_PHY_GENERATION3);
> >> + if (ret < 0)
> >> + goto err4;
> >
> > Do you have any warranties that phy callbacks won't get called before
> > i2c device probes and sets i2c_client?
> >
> >> +
> >> + ret = i2c_add_driver(&sataphy_i2c_driver);
> >> + if (ret < 0)
> >> + goto err5;
> >> +
> >> + platform_set_drvdata(pdev, phy);
>
> I shall register my i2c client driver before assigning the SATA PHY
> call backs. Would that guarantee the calling of phy callbacks after
> the i2c device probes?
No. You might still not have the sata phy i2c controller registered at
that time and so the i2c driver will not be probed.
The only definitive solution for this would be to register both platform
and i2c drivers in module init (instead of using module_platform_driver)
and defer the probe of platform driver until i2c device gets registered.
You can defer the probe by returning -EPROBE_DEFER after checking that
i2c_client is still NULL.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121017/ff347413/attachment-0001.html>
^ permalink raw reply [flat|nested] 42+ messages in thread