Adding support for SATA in EXYNO5
Subsystems (6)
cover Adding support for SATA in EXYNO5
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
Signature · 1 line
1.7.5.4
PATCH 1/6 ARM: EXYNOS5: Clock settings for SATA and SATA PHY
This patch adds neccessary clock entries for SATA, SATA PHY and I2C_SATAPHY
--- arch/arm/mach-exynos/clock-exynos5.c | 19 +++++++++++++++++-- 1 files changed, 17 insertions(+), 2 deletions(-)
arch/arm/mach-exynos/clock-exynos5.c +17 -2
@@ -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,
Signature · 1 line
1.7.5.4
PATCH 2/6 ARM: EXYNOS5: DT Support for SATA and SATA PHY
In reply to Vasanth AnanthanThis patch adds Device Nodes for SATA and SATA PHY device.
--- 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(-)
arch/arm/boot/dts/exynos5250-smdk5250.dts +11 -0
@@ -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"; };
arch/arm/boot/dts/exynos5250.dtsi +20 -0
@@ -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>; }; }; + };
arch/arm/mach-exynos/include/mach/map.h +7 -0
@@ -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
arch/arm/mach-exynos/mach-exynos5-dt.c +6 -0
@@ -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,
Signature · 1 line
1.7.5.4
PATCH 2/6 ARM: EXYNOS5: DT Support for SATA and SATA PHY
Hi,
Quoted 15 lines · Vasanth Ananthan
This patch adds Device Nodes for SATA and SATA PHY device.
Signed-off-by Vasanth Ananthan <[email protected]>--- 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(-)arch/arm/boot/dts/exynos5250-smdk5250.dts +0
-0
arch/arm/boot/dts/exynos5250-smdk5250.dts line 54
Quoted 7 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +4
-0@@ -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.
arch/arm/boot/dts/exynos5250-smdk5250.dts line 56
Quoted 2 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +2
-0+ + sataphy@70 {
sata-phy would be a more conventional name.
arch/arm/boot/dts/exynos5250-smdk5250.dts line 57
Quoted 1 line
arch/arm/boot/dts/exynos5250-smdk5250.dts +1
-0+ compatible = "samsung,i2c-phy";
i2c-phy? Seems like an odd choice of name. What is this device?
arch/arm/boot/dts/exynos5250-smdk5250.dts line 58
Quoted 1 line
arch/arm/boot/dts/exynos5250-smdk5250.dts +1
-0+ 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.
Quoted 11 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +3
-0+ }; + }; + i2c@12C80000 { status = "disabled"; };arch/arm/boot/dts/exynos5250.dtsi +0
-0
arch/arm/boot/dts/exynos5250.dtsi line 94
Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi +4
-0@@ -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.
arch/arm/boot/dts/exynos5250.dtsi line 100
Quoted 6 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ }; + + sata-phy@12170000 { + compatible = "samsung,exynos-sata-phy"; + reg = <0x12170000 0x1ff>; + };
Should have binding too. How does this relate to the i2c device above.
arch/arm/boot/dts/exynos5250.dtsi line 168
Quoted 10 lines
arch/arm/boot/dts/exynos5250.dtsi +3
-0+ 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.
arch/arm/boot/dts/exynos5250.dtsi line 482
Quoted 13 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ 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.
arch/arm/mach-exynos/include/mach/map.h line 180
Quoted 10 lines
arch/arm/boot/dts/exynos5250.dtsi +0
-0};arch/arm/mach-exynos/include/mach/map.h +1
-0@@ -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.
arch/arm/mach-exynos/include/mach/map.h line 185
Quoted 5 lines
arch/arm/mach-exynos/include/mach/map.h +2
-0#define EXYNOS4_PA_SATA 0x12560000#define EXYNOS4_PA_SATAPHY 0x125D0000#define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000 +#endif +#ifdef CONFIG_ARCH_EXYNOS5
Same here.
arch/arm/mach-exynos/include/mach/map.h line 189
Quoted 4 lines
arch/arm/mach-exynos/include/mach/map.h +4
-0+#define EXYNOS5_PA_SATA_PHY_CTRL 0x12170000 +#define EXYNOS5_PA_SATA_PHY_I2C 0x121D0000 +#define EXYNOS5_PA_SATA_BASE 0x122F0000 +#endif
-Olof
PATCH 2/6 ARM: EXYNOS5: DT Support for SATA and SATA PHY
Hi,
Quoted 30 lines · Olof Johansson
Hi,
Review
Quoted 11 lines · Vasanth Ananthan
This patch adds Device Nodes for SATA and SATA PHY device.
Signed-off-by Vasanth Ananthan <[email protected]>--- 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(-)(quoted diff) +0
-0
b/arch/arm/boot/dts/exynos5250-smdk5250.dtsarch/arm/boot/dts/exynos5250-smdk5250.dts line 54
Quoted 11 lines
index 8a5e348..bb262ce 100644
arch/arm/boot/dts/exynos5250-smdk5250.dts +4
-0@@ -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.
Quoted 9 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts line 56
Quoted 2 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +2
-0+ + sataphy at 70 {sata-phy would be a more conventional name.
arch/arm/boot/dts/exynos5250-smdk5250.dts line 57
Quoted 1 line
arch/arm/boot/dts/exynos5250-smdk5250.dts +1
-0+ 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.
Quoted 30 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts line 58
Quoted 2 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +1
-0+ 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.Review
Quoted 7 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +3
-0+ }; + }; + i2c at 12C80000 { status = "disabled"; };(quoted diff) +0
-0
b/arch/arm/boot/dts/exynos5250.dtsiarch/arm/boot/dts/exynos5250.dtsi line 94
Quoted 11 lines
index 004aaa8..5a47a8f 100644
arch/arm/boot/dts/exynos5250.dtsi +4
-0@@ -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.
Quoted 9 lines
arch/arm/boot/dts/exynos5250.dtsi line 100
Quoted 6 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ }; + + 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.
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250.dtsi +1
-0+ i2c at 12C60000 { compatible = "samsung,s3c2440-i2c"; reg = <0x12C60000 0x100>;@@ -152,6 +164,13 @@ #size-cells = <0>; };
Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi line 168
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +2
-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.
Quoted 38 lines
arch/arm/boot/dts/exynos5250.dtsi line 482
Quoted 13 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ 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.
Review
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +0
-0};(quoted diff) +0
-0
b/arch/arm/mach-exynos/include/mach/map.harch/arm/mach-exynos/include/mach/map.h line 180
Quoted 8 lines
index c72b675..6827190 100644
arch/arm/mach-exynos/include/mach/map.h +1
-0@@ -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.
arch/arm/mach-exynos/include/mach/map.h line 185
Quoted 5 lines
arch/arm/mach-exynos/include/mach/map.h +2
-0#define EXYNOS4_PA_SATA 0x12560000#define EXYNOS4_PA_SATAPHY 0x125D0000#define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000 +#endif +#ifdef CONFIG_ARCH_EXYNOS5 Same here.
Quoted 8 lines
arch/arm/mach-exynos/include/mach/map.h line 189
Quoted 4 lines
arch/arm/mach-exynos/include/mach/map.h +4
-0+#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.
Signature · 4 lines
Vasanth Ananthan. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121010/7856e75d/attachment-0001.html>
PATCH 2/6 ARM: EXYNOS5: DT Support for SATA and SATA PHY
Hi,
Quoted 30 lines · Olof Johansson
Hi,
Quoted 15 lines · Vasanth Ananthan
This patch adds Device Nodes for SATA and SATA PHY device.
Signed-off-by Vasanth Ananthan <[email protected]>--- 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(-)arch/arm/boot/dts/exynos5250-smdk5250.dts +0
-0arch/arm/boot/dts/exynos5250-smdk5250.dts line 54
Quoted 7 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +4
-0@@ -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.
Quoted 8 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts line 56
Quoted 2 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +2
-0+ + sataphy at 70 {sata-phy would be a more conventional name.
arch/arm/boot/dts/exynos5250-smdk5250.dts line 57
Quoted 1 line
arch/arm/boot/dts/exynos5250-smdk5250.dts +1
-0+ 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.
Quoted 29 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts line 58
Quoted 3 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +1
-0+ 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.Quoted 11 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +3
-0+ }; + }; + i2c at 12C80000 { status = "disabled"; };arch/arm/boot/dts/exynos5250.dtsi +0
-0arch/arm/boot/dts/exynos5250.dtsi line 94
Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi +4
-0@@ -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.
Quoted 8 lines
arch/arm/boot/dts/exynos5250.dtsi line 100
Quoted 6 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ }; + + 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.
Quoted 9 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250.dtsi +1
-0+ i2c at 12C60000 { compatible = "samsung,s3c2440-i2c"; reg = <0x12C60000 0x100>;@@ -152,6 +164,13 @@ #size-cells = <0>; };
Quoted 5 lines
arch/arm/boot/dts/exynos5250.dtsi line 168
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +2
-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.
Quoted 38 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250.dtsi +5
-0+ reg = <0x121D0000 0x100>; + #address-cells = <1>; + #size-cells = <0>; + }; + spi_0: spi at 12d20000 { compatible = "samsung,exynos4210-spi"; reg = <0x12d20000 0x100>;arch/arm/boot/dts/exynos5250.dtsi line 482
Quoted 4 lines
arch/arm/boot/dts/exynos5250.dtsi +1
-0@@ -460,4 +479,5 @@ #gpio-cells = <4>; }; }; +Stray whitespace change.
arch/arm/mach-exynos/include/mach/map.h line 180
Quoted 10 lines
arch/arm/boot/dts/exynos5250.dtsi +0
-0};arch/arm/mach-exynos/include/mach/map.h +1
-0@@ -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.
arch/arm/mach-exynos/include/mach/map.h line 185
Quoted 5 lines
arch/arm/mach-exynos/include/mach/map.h +2
-0#define EXYNOS4_PA_SATA 0x12560000#define EXYNOS4_PA_SATAPHY 0x125D0000#define EXYNOS4_PA_SATAPHY_CTRL 0x126B0000 +#endif +#ifdef CONFIG_ARCH_EXYNOS5 Same here.
Quoted 7 lines
arch/arm/mach-exynos/include/mach/map.h line 189
Quoted 4 lines
arch/arm/mach-exynos/include/mach/map.h +4
-0+#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.
PATCH 2/6 ARM: EXYNOS5: DT Support for SATA and SATA PHY
In reply to Vasanth AnanthanHi,
Quoted 7 lines · Vasanth Ananthan
Quoted 7 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts line 56
Quoted 2 lines
arch/arm/boot/dts/exynos5250-smdk5250.dts +2
-0+ + sataphy@70 {sata-phy would be a more conventional name.
Quoted 1 line
+ compatible = "samsung,i2c-phy";
Quoted 7 lines
Quoted 2 lines
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).
Quoted 10 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250.dtsi line 100
Quoted 6 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ }; + + sata-phy@12170000 { + compatible = "samsung,exynos-sata-phy"; + reg = <0x12170000 0x1ff>; + };Should have binding too. How does this relate to the i2c device above.
Quoted 6 lines
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.
Quoted 16 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/boot/dts/exynos5250.dtsi +1
-0+ i2c@12C60000 { compatible = "samsung,s3c2440-i2c"; reg = <0x12C60000 0x100>;@@ -152,6 +164,13 @@ #size-cells = <0>; };Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi line 168
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +2
-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.
Quoted 4 lines
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
PATCH 2/6 ARM: EXYNOS5: DT Support for SATA and SATA PHY
Hi,
Quoted 14 lines · Olof Johansson
Hi,
Quoted 10 lines · Vasanth Ananthan
Quoted 9 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ + sataphy at 70 {sata-phy would be a more conventional name.
Quoted 1 line
+ compatible = "samsung,i2c-phy";
i2c-phy? Seems like an odd choice of name. What is this device?
Quoted 9 lines
Quoted 4 lines
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..
Quoted 18 lines
Quoted 11 lines
Quoted 10 lines
arch/arm/boot/dts/exynos5250.dtsi line 100
Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi +6
-0+ }; + + 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.
Quoted 6 lines
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.
Quoted 3 lines
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.
Quoted 22 lines
Quoted 17 lines
Quoted 9 lines
Quoted 9 lines
arch/arm/boot/dts/exynos5250.dtsi +1
-0+ i2c at 12C60000 { compatible = "samsung,s3c2440-i2c"; reg = <0x12C60000 0x100>;@@ -152,6 +164,13 @@ #size-cells = <0>; };Quoted 7 lines
Review line 168
Quoted 2 lines
(quoted diff) +2
-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.Quoted 4 lines
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.
Quoted 9 lines
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.
Quoted 2 lines
-Olof
Signature · 3 lines
Regards, Vasanth K A
PATCH 3/6 DRIVERS: ATA: SATA PHY utility framework
In reply to Vasanth AnanthanThis 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.
--- 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
drivers/ata/Kconfig +9 -0
@@ -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
drivers/ata/Makefile +1 -0
@@ -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
drivers/ata/sata_phy.c new file +99 -0
99 lines not on this page · raw message
drivers/ata/sata_phy.h new file +44 -0
44 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 3/6 DRIVERS: ATA: SATA PHY utility framework
Hi Vasanth,
Quoted 41 lines · 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 <[email protected]>--- 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.hdrivers/ata/Kconfig +9
-0@@ -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_SOCdrivers/ata/Makefile +0
-0
drivers/ata/Makefile obj-$(CONFIG_SATA_FSL) += sata_fsl.o · line 12
Quoted 4 lines
drivers/ata/Makefile +1
-0@@ -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?
Quoted 45 lines
drivers/ata/Makefile +0
-0# SFF w/ custom DMA obj-$(CONFIG_PDC_ADMA) += pdc_adma.odrivers/ata/sata_phy.c new file +8
-0@@ -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.
drivers/ata/sata_phy.c new file +27
-0+*/ + +#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 ; + } + } +
drivers/ata/sata_phy.c line 46
Quoted 10 lines
drivers/ata/sata_phy.c new file +10
-0+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?
drivers/ata/sata_phy.c
Quoted 8 lines
drivers/ata/sata_phy.c new file +8
-0+ +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");
Quoted 8 lines
drivers/ata/sata_phy.c new file +8
-0+ goto out; + } + } + phy->type = type; +list_add_tail (&phy->head, &phy_list); + ret =0 ; + } +
drivers/ata/sata_phy.c line 72
Quoted 10 lines
drivers/ata/sata_phy.c new file +10
-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
PATCH 3/6 DRIVERS: ATA: SATA PHY utility framework
Hi Tomasz,
Quoted 64 lines · Tomasz Figa
Hi Vasanth,
Quoted 41 lines · 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 <[email protected]>--- 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.hdrivers/ata/Kconfig +9
-0@@ -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_SOCdrivers/ata/Makefile +0
-0drivers/ata/Makefile obj-$(CONFIG_SATA_FSL) += sata_fsl.o · line 12
Quoted 4 lines
drivers/ata/Makefile +1
-0@@ -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.oIf the framework introduced by this patch is supposed to be generic, maybe a new
Kconfigentry should be created for it, likeCONFIG_SATA_PHY, which would be selected by any drivers using it?drivers/ata/Makefile obj-$(CONFIG_SATA_FSL) += sata_fsl.o
Quoted 5 lines
drivers/ata/Makefile +5
-0+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:
Quoted 5 lines
if (!phy) return -EINVAL;and then make the code below unconditional?
I'll incorporate the suggestions.
Signature · 1 line
Vasanth
PATCH 4/6 ARM: S3C2410: I2C driver polling mode support
In reply to Vasanth AnanthanThis 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.
--- drivers/i2c/busses/i2c-s3c2410.c | 84 +++++++++++++++++++++++++++++--------- 1 files changed, 65 insertions(+), 19 deletions(-)
drivers/i2c/busses/i2c-s3c2410.c +65 -19
@@ -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);
Signature · 1 line
1.7.5.4
PATCH 4/6 ARM: S3C2410: I2C driver polling mode support
Hi,
Quoted 34 lines · 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_SATAPHYcontroller lacks an interrupt line but the s3c-2410 driver is interrupt driven. Hence this support is required for functioning of theI2C_SATAPHYcontroller.Signed-off-by Vasanth Ananthan <[email protected]>--- drivers/i2c/busses/i2c-s3c2410.c | 84+++++++++++++++++++++++++++++--------- 1 files changed, 65 insertions(+), 19 deletions(-)
(quoted diff) +0
-0
b/drivers/i2c/busses/i2c-s3c2410.cindex 5ae3b02..699b530 100644drivers/i2c/busses/i2c-s3c2410.c +2
-0@@ -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 longiicstat); +
#ifdefCONFIG_OF
static const structof_device_ids3c24xx_i2c_match[] = {drivers/i2c/busses/i2c-s3c2410.c +0
-0{ .compatible = "samsung,s3c2410-i2c", .data = (void *)0 }, { .compatible = "samsung,s3c2440-i2c", .data = (void *)QUIRK_S3C2440}, + { .compatible = "samsung,s3c2440-sataphy-i2c",
drivers/i2c/busses/i2c-s3c2410.c static struct platform_device_id s3c24xx_driver_ids[] = { · line 113
Quoted 1 line
drivers/i2c/busses/i2c-s3c2410.c +1
-0+ .data = (void *)(QUIRK_S3C2440|QUIRK_SATAPHY|QUIRK_NO_GPIO) },
nitpick: Please insert spaces around bitwise OR operators.
Quoted 14 lines
drivers/i2c/busses/i2c-s3c2410.c +2
-1{ .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)
drivers/i2c/busses/i2c-s3c2410.c static inline void s3c24xx_i2c_enable_irq(struct · line 195
Quoted 6 lines
drivers/i2c/busses/i2c-s3c2410.c +3
-0@@ -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.
drivers/i2c/busses/i2c-s3c2410.c static inline void s3c24xx_i2c_enable_irq(struct s3c24xx_i2c *i2c) · line 204
Quoted 9 lines
drivers/i2c/busses/i2c-s3c2410.c +9
-0+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.
Quoted 7 lines
drivers/i2c/busses/i2c-s3c2410.c +3
-0+return false; +} +/* s3c24xx_i2c_message_start * * put the start of a message onto the bus
drivers/i2c/busses/i2c-s3c2410.c static void s3c24xx_i2c_message_start(struct · line 254
Quoted 7 lines
drivers/i2c/busses/i2c-s3c2410.c +3
-0@@ -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.
drivers/i2c/busses/i2c-s3c2410.c static void s3c24xx_i2c_message_start(struct · line 256
Quoted 2 lines
drivers/i2c/busses/i2c-s3c2410.c +2
-0+while ((i2c->msg_num !=0 ) &&is_ack (i2c)) { +
Same here.
drivers/i2c/busses/i2c-s3c2410.c static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, · line 261
Quoted 6 lines
drivers/i2c/busses/i2c-s3c2410.c +6
-0+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.
drivers/i2c/busses/i2c-s3c2410.c static void s3c24xx_i2c_message_start(struct · line 264
Quoted 2 lines
drivers/i2c/busses/i2c-s3c2410.c +2
-0+ } +
Same here.
drivers/i2c/busses/i2c-s3c2410.c static void s3c24xx_i2c_message_start(struct · line 266
Quoted 2 lines
drivers/i2c/busses/i2c-s3c2410.c +2
-0+ } +
Same here.
drivers/i2c/busses/i2c-s3c2410.c static void s3c24xx_i2c_message_start(struct · line 268
Quoted 2 lines
drivers/i2c/busses/i2c-s3c2410.c +0
-0}
Best regards,
Tomasz Figa
PATCH 4/6 ARM: S3C2410: I2C driver polling mode support
In reply to Vasanth AnanthanAm Dienstag, 9. Oktober 2012, 13:48:50 schrieb Vasanth Ananthan:
Quoted 15 lines
This patch adds polling mode support for i2c s3c-2410 driver. The
I2C_SATAPHYcontroller lacks an interrupt line but the s3c-2410 driver is interrupt driven. Hence this support is required for functioning of theI2C_SATAPHYcontroller.Signed-off-by Vasanth Ananthan <[email protected]>--- drivers/i2c/busses/i2c-s3c2410.c | 84+++++++++++++++++++++++++++++--------- 1 files changed, 65 insertions(+), 19 deletions(-)
(quoted diff) +0
-0
b/drivers/i2c/busses/i2c-s3c2410.cindex 5ae3b02..699b530 100644drivers/i2c/busses/i2c-s3c2410.c +0
-0
[...]
Quoted 8 lines
drivers/i2c/busses/i2c-s3c2410.c +1
-0@@ -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 longiicstat); +
#ifdefCONFIG_OF
static const structof_device_ids3c24xx_i2c_match[] = {
drivers/i2c/busses/i2c-s3c2410.c static struct platform_device_id s3c24xx_driver_ids[] = { · line 116
Quoted 7 lines
drivers/i2c/busses/i2c-s3c2410.c +2
-0{ .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
PATCH 5/6 ARM: EYNOS5: SATA controller driver
In reply to Vasanth AnanthanThis patch adds a platform driver for SATA controller.
--- 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
arch/arm/mach-exynos/include/mach/regs-pmu.h +3 -0
@@ -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 */
drivers/ata/Makefile +1 -1
@@ -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
drivers/ata/sata_exynos.c new file +274 -0
274 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 5/6 ARM: EYNOS5: SATA controller driver
Hi,
Quoted 69 lines · 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 <[email protected]>--- 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(quoted diff) +0
-0
b/arch/arm/mach-exynos/include/mach/regs-pmu.hindex d4e392b..3716ce8 100644arch/arm/mach-exynos/include/mach/regs-pmu.h +3
-0@@ -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 */ drivers/ata/Makefile +1
-1@@ -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.olibahci.o
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.odrivers/ata/sata_exynos.c new file +8
-0@@ -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.
drivers/ata/sata_exynos.c new file +16
-0+*/ + +#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"
drivers/ata/sata_exynos.c line 35
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+ +#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.
Quoted 15 lines
drivers/ata/sata_exynos.c new file +15
-0+}; + +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) +{
drivers/ata/sata_exynos.c line 60
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+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?
drivers/ata/sata_exynos.c line 64
Quoted 4 lines
drivers/ata/sata_exynos.c new file +4
-0+if (!sata) { +dev_err (dev,"can't alloc sata\n" ); + ret = -ENOMEM ; +return ret;
return -ENOMEM;
Quoted 23 lines
drivers/ata/sata_exynos.c new file +23
-0+ } + + 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;
drivers/ata/sata_exynos.c line 97
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+ } + + 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?
drivers/ata/sata_exynos.c line 107
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+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?
Quoted 70 lines
drivers/ata/sata_exynos.c new file +70
-0+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;
drivers/ata/sata_exynos.c
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+ } + + 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,
Quoted 47 lines
drivers/ata/sata_exynos.c new file +47
-0+ &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); +
drivers/ata/sata_exynos.c line 244
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+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.
drivers/ata/sata_exynos.c line 255
Quoted 11 lines
drivers/ata/sata_exynos.c new file +11
-0+}; + +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.
drivers/ata/sata_exynos.c
Quoted 5 lines
drivers/ata/sata_exynos.c new file +5
-0+}; + +static int __init exynos_sata_init (void ) +{ + return platform_driver_probe(&exynos_sata_driver,
exynos_sata_probe);
drivers/ata/sata_exynos.c line 270
Quoted 10 lines
drivers/ata/sata_exynos.c new file +10
-0+} + +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
PATCH 6/6 ARM: EXYNOS5: SATA PHY controller driver
In reply to Vasanth AnanthanThis patch adds a platform driver and I2C client driver for SATA PHY controller
--- 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
drivers/ata/Makefile +1 -1
@@ -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
drivers/ata/sata_exynos_phy.c new file +303 -0
303 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 6/6 ARM: EXYNOS5: SATA PHY controller driver
Hi,
Quoted 76 lines · 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 <[email protected]>--- 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.cdrivers/ata/Makefile +0
-1@@ -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.olibahci.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(quoted diff) +0
-0
b/drivers/ata/sata_exynos_phy.cnew file mode 100644 index 0000000..44861dedrivers/ata/sata_exynos_phy.c new file +8
-0@@ -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.
drivers/ata/sata_exynos_phy.c new file +36
-0+*/ + +#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
Quoted 1 line
Status) +{
Please use lowercase for variable names.
Quoted 8 lines
drivers/ata/sata_exynos_phy.c new file +7
-0+if ((readl (base + reg) & checkbit) == Status) +return true; +else +return false; +} + +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32checkbit, + u32 Status)
Same here.
drivers/ata/sata_exynos_phy.c line 66
Quoted 12 lines
drivers/ata/sata_exynos_phy.c new file +12
-0+{ +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) ?
Quoted 56 lines
drivers/ata/sata_exynos_phy.c new file +56
-0+{ +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 ;
drivers/ata/sata_exynos_phy.c line 132
Quoted 10 lines
drivers/ata/sata_exynos_phy.c new file +10
-0+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) ?
Quoted 16 lines
drivers/ata/sata_exynos_phy.c new file +16
-0+{ + +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 ;
drivers/ata/sata_exynos_phy.c line 158
Quoted 10 lines
drivers/ata/sata_exynos_phy.c new file +10
-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_
drivers/ata/sata_exynos_phy.c line 165
Quoted 7 lines
drivers/ata/sata_exynos_phy.c new file +7
-0+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_
Quoted 5 lines
drivers/ata/sata_exynos_phy.c new file +5
-0+if (!sataphy) { +dev_err (&pdev->dev,"failed to allocate memory\n" ); + ret = -ENOMEM ; + goto err0; + }
drivers/ata/sata_exynos_phy.c line 180
Quoted 10 lines
drivers/ata/sata_exynos_phy.c new file +10
-0+ + 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_
drivers/ata/sata_exynos_phy.c line 188
Quoted 8 lines
drivers/ata/sata_exynos_phy.c new file +8
-0+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_
Quoted 1 line
+
nitpick: Unnecessary blank line.
drivers/ata/sata_exynos_phy.c line 196
Quoted 7 lines
drivers/ata/sata_exynos_phy.c new file +7
-0+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_
Quoted 5 lines
drivers/ata/sata_exynos_phy.c new file +5
-0+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;
drivers/ata/sata_exynos_phy.c line 211
Quoted 10 lines
drivers/ata/sata_exynos_phy.c new file +10
-0+ } + + 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?
Quoted 46 lines
drivers/ata/sata_exynos_phy.c new file +46
-0+ + 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);
drivers/ata/sata_exynos_phy.c line 267
Quoted 10 lines
drivers/ata/sata_exynos_phy.c new file +10
-0+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.
drivers/ata/sata_exynos_phy.c line 274
Quoted 7 lines
drivers/ata/sata_exynos_phy.c new file +7
-0+ {}, +}; + +MODULE_DEVICE_TABLE (of, sata_phy_of_match); + +static const struct i2c_device_id phy_i2c_device_match[] = { + {"sataphy" ,0 },
Same here.
drivers/ata/sata_exynos_phy.c line 286
Quoted 12 lines
drivers/ata/sata_exynos_phy.c new file +12
-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, + },
nitpick: Wrong indentation of closing bracket.
drivers/ata/sata_exynos_phy.c line 294
Quoted 8 lines
drivers/ata/sata_exynos_phy.c new file +8
-0+}; + +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
PATCH 6/6 ARM: EXYNOS5: SATA PHY controller driver
Hi Tomasz,
Quoted 14 lines · Tomasz Figa
Hi,
Quoted 11 lines · 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 <[email protected]>--- 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
[ ... ]
Quoted 13 lines
drivers/ata/sata_exynos_phy.c line 211
Quoted 9 lines
drivers/ata/sata_exynos_phy.c new file +9
-0+ + 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?
Quoted 6 lines
drivers/ata/sata_exynos_phy.c line 217
Quoted 6 lines
drivers/ata/sata_exynos_phy.c new file +6
-0+ + 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
PATCH 6/6 ARM: EXYNOS5: SATA PHY controller driver
Hi Vasanth,
Quoted 3 lines · 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 <[email protected]>
Quoted 32 lines · wrote:
Quoted 16 lines
Hi,
Quoted 13 lines · 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 <[email protected]>---
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[ ... ]
Quoted 13 lines
drivers/ata/sata_exynos_phy.c line 211
Quoted 9 lines
drivers/ata/sata_exynos_phy.c new file +9
-0+ + 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?
Quoted 10 lines
Quoted 6 lines
drivers/ata/sata_exynos_phy.c line 217
Quoted 6 lines
drivers/ata/sata_exynos_phy.c new file +6
-0+ + 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,
Signature · 2 lines
Tomasz Figa Samsung Poland R&D Center
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 6/6 ARM: EXYNOS5: SATA PHY controller driver
In reply to Vasanth AnanthanHi Vasanth,
Quoted 3 lines · 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 <[email protected]>
Quoted 32 lines · wrote:
Quoted 16 lines
Hi,
Quoted 13 lines · 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 <[email protected]>---
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[ ... ]
Quoted 13 lines
drivers/ata/sata_exynos_phy.c line 211
Quoted 9 lines
drivers/ata/sata_exynos_phy.c new file +9
-0+ + 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?
Quoted 10 lines
Quoted 6 lines
drivers/ata/sata_exynos_phy.c line 217
Quoted 6 lines
drivers/ata/sata_exynos_phy.c new file +6
-0+ + 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,
Signature · 2 lines
Tomasz Figa Samsung Poland R&D Center
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel