Re: [PATCH] ARM: SAMSUNG: devs: Add names to fimd0 IRQ resources

2013-05-16 Thread Jingoo Han
Thursday, May 16, 2013 12:10 AM, Tomasz Figa wrote:
 
 Since commit 1977e6d8 (drm/exynos: change the method for getting the
 interrupt) the Exynos DRM FIMD driver requires IRQ resources to be
 named. This patch fixes probe failure in non-DT cases by adding
 appropriate resource names to fimd0 platform device.
 
 Signed-off-by: Tomasz Figa t.f...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

It looks good.
Thank you for sending the patch.

Acked-by: Jingoo Han jg1@samsung.com


Best regards,
Jingoo Han

 ---
  arch/arm/plat-samsung/devs.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
 index 30c2fe2..0f9c3f4 100644
 --- a/arch/arm/plat-samsung/devs.c
 +++ b/arch/arm/plat-samsung/devs.c
 @@ -311,9 +311,9 @@ struct platform_device s5p_device_jpeg = {
  #ifdef CONFIG_S5P_DEV_FIMD0
  static struct resource s5p_fimd0_resource[] = {
   [0] = DEFINE_RES_MEM(S5P_PA_FIMD0, SZ_32K),
 - [1] = DEFINE_RES_IRQ(IRQ_FIMD0_VSYNC),
 - [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO),
 - [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM),
 + [1] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_VSYNC, vsync),
 + [2] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_FIFO, fifo),
 + [3] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_SYSTEM, lcd_sys),
  };
 
  struct platform_device s5p_device_fimd0 = {
 --
 1.8.2.1
 



[PATCH v2 0/6] Samsung USB PHY SoC support cleanup

2013-05-16 Thread Tomasz Figa
This patch series intends to improve handling of SoC-specific differences
in Samsung USB PHY drivers, by reducing the need to explicitly check
SoC type using if and switch statements.

In addition, last patch adds support for Exynos 4x12, as this is simply
a matter of adding appropriate driver data and additional case in two switches.

Tested on Exynos4210-based Trats board and Exynos4412-based internal
Samsung board.

Changes since v1:
 - rebased to latest usb-next

Tomasz Figa (6):
  usb: phy: samsung: Select common driver part implicitly
  usb: phy: samsung: Use clk_get to get reference clock
  usb: phy: samsung: Consolidate reference clock rate handling
  usb: phy: samsung: Pass set_isolation callback through driver data
  usb: phy: samsung: Pass enable/disable callbacks through driver data
  usb: phy: samsung: Add support for USB 2.0 PHY on Exynos 4x12

 drivers/usb/phy/Kconfig|   2 +-
 drivers/usb/phy/phy-samsung-usb.c  | 154 ++---
 drivers/usb/phy/phy-samsung-usb.h  |  14 +++-
 drivers/usb/phy/phy-samsung-usb2.c |  53 +
 drivers/usb/phy/phy-samsung-usb3.c |  23 --
 5 files changed, 146 insertions(+), 100 deletions(-)

-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/6] usb: phy: samsung: Select common driver part implicitly

2013-05-16 Thread Tomasz Figa
Since phy-samsung-usb library can be used only by phy-samsung-usb2 and
phy-samsung-usb3 drivers, there is no need to give explicit control over
its Kconfig symbol.

This patch makes CONFIG_SAMSUNG_USBPHY symbol hidden and selected
implicitly by CONFIG_SAMSUNG_USB2PHY and CONFIG_SAMSUNG_USB3PHY.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 371d0e7..a2b9e18 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -86,7 +86,7 @@ config OMAP_USB3
  on/off the PHY.
 
 config SAMSUNG_USBPHY
-   tristate Samsung USB PHY Driver
+   tristate
help
  Enable this to support Samsung USB phy helper driver for Samsung SoCs.
  This driver provides common interface to interact, for Samsung USB 
2.0 PHY
-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/6] usb: phy: samsung: Use clk_get to get reference clock

2013-05-16 Thread Tomasz Figa
There is no need to use devm_clk_get to get a clock that is being put
at the end of the function.

This patch changes the code getting reference clock to use clk_get
instead of useless in this case devm_clk_get.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/phy/phy-samsung-usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-samsung-usb.c 
b/drivers/usb/phy/phy-samsung-usb.c
index 7b118ee5..62cdb7e 100644
--- a/drivers/usb/phy/phy-samsung-usb.c
+++ b/drivers/usb/phy/phy-samsung-usb.c
@@ -175,9 +175,9 @@ int samsung_usbphy_get_refclk_freq(struct samsung_usbphy 
*sphy)
 * external crystal clock XXTI
 */
if (sphy-drv_data-cpu_type == TYPE_EXYNOS5250)
-   ref_clk = devm_clk_get(sphy-dev, ext_xtal);
+   ref_clk = clk_get(sphy-dev, ext_xtal);
else
-   ref_clk = devm_clk_get(sphy-dev, xusbxti);
+   ref_clk = clk_get(sphy-dev, xusbxti);
if (IS_ERR(ref_clk)) {
dev_err(sphy-dev, Failed to get reference clock\n);
return PTR_ERR(ref_clk);
-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 4/6] usb: phy: samsung: Pass set_isolation callback through driver data

2013-05-16 Thread Tomasz Figa
This patch extends driver data structure with set_isolation callback,
which allows to remove the need for checking for SoC type in a switch
statement.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/phy/phy-samsung-usb.c  | 36 
 drivers/usb/phy/phy-samsung-usb.h  |  4 +++-
 drivers/usb/phy/phy-samsung-usb2.c | 11 +++
 drivers/usb/phy/phy-samsung-usb3.c |  7 +--
 4 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/phy/phy-samsung-usb.c 
b/drivers/usb/phy/phy-samsung-usb.c
index c40ea32..7a1ed90 100644
--- a/drivers/usb/phy/phy-samsung-usb.c
+++ b/drivers/usb/phy/phy-samsung-usb.c
@@ -73,7 +73,7 @@ EXPORT_SYMBOL_GPL(samsung_usbphy_parse_dt);
  * Here 'on = true' would mean USB PHY block is isolated, hence
  * de-activated and vice-versa.
  */
-void samsung_usbphy_set_isolation(struct samsung_usbphy *sphy, bool on)
+void samsung_usbphy_set_isolation_4210(struct samsung_usbphy *sphy, bool on)
 {
void __iomem *reg = NULL;
u32 reg_val;
@@ -84,32 +84,12 @@ void samsung_usbphy_set_isolation(struct samsung_usbphy 
*sphy, bool on)
return;
}
 
-   switch (sphy-drv_data-cpu_type) {
-   case TYPE_S3C64XX:
-   /*
-* Do nothing: We will add here once S3C64xx goes for DT support
-*/
-   break;
-   case TYPE_EXYNOS4210:
-   /*
-* Fall through since exynos4210 and exynos5250 have similar
-* register architecture: two separate registers for host and
-* device phy control with enable bit at position 0.
-*/
-   case TYPE_EXYNOS5250:
-   if (sphy-phy_type == USB_PHY_TYPE_DEVICE) {
-   reg = sphy-pmuregs +
-   sphy-drv_data-devphy_reg_offset;
-   en_mask = sphy-drv_data-devphy_en_mask;
-   } else if (sphy-phy_type == USB_PHY_TYPE_HOST) {
-   reg = sphy-pmuregs +
-   sphy-drv_data-hostphy_reg_offset;
-   en_mask = sphy-drv_data-hostphy_en_mask;
-   }
-   break;
-   default:
-   dev_err(sphy-dev, Invalid SoC type\n);
-   return;
+   if (sphy-phy_type == USB_PHY_TYPE_DEVICE) {
+   reg = sphy-pmuregs + sphy-drv_data-devphy_reg_offset;
+   en_mask = sphy-drv_data-devphy_en_mask;
+   } else if (sphy-phy_type == USB_PHY_TYPE_HOST) {
+   reg = sphy-pmuregs + sphy-drv_data-hostphy_reg_offset;
+   en_mask = sphy-drv_data-hostphy_en_mask;
}
 
reg_val = readl(reg);
@@ -121,7 +101,7 @@ void samsung_usbphy_set_isolation(struct samsung_usbphy 
*sphy, bool on)
 
writel(reg_val, reg);
 }
-EXPORT_SYMBOL_GPL(samsung_usbphy_set_isolation);
+EXPORT_SYMBOL_GPL(samsung_usbphy_set_isolation_4210);
 
 /*
  * Configure the mode of working of usb-phy here: HOST/DEVICE.
diff --git a/drivers/usb/phy/phy-samsung-usb.h 
b/drivers/usb/phy/phy-samsung-usb.h
index 0336f6b..5203784 100644
--- a/drivers/usb/phy/phy-samsung-usb.h
+++ b/drivers/usb/phy/phy-samsung-usb.h
@@ -271,6 +271,7 @@ struct samsung_usbphy_drvdata {
u32 devphy_reg_offset;
u32 hostphy_reg_offset;
int (*rate_to_clksel)(struct samsung_usbphy *, unsigned long);
+   void (*set_isolation)(struct samsung_usbphy *, bool);
 };
 
 /*
@@ -323,7 +324,8 @@ static inline const struct samsung_usbphy_drvdata
 }
 
 extern int samsung_usbphy_parse_dt(struct samsung_usbphy *sphy);
-extern void samsung_usbphy_set_isolation(struct samsung_usbphy *sphy, bool on);
+extern void samsung_usbphy_set_isolation_4210(struct samsung_usbphy *sphy,
+   bool on);
 extern void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy);
 extern int samsung_usbphy_set_type(struct usb_phy *phy,
enum samsung_usb_phy_type phy_type);
diff --git a/drivers/usb/phy/phy-samsung-usb2.c 
b/drivers/usb/phy/phy-samsung-usb2.c
index 802e738..ae6da68 100644
--- a/drivers/usb/phy/phy-samsung-usb2.c
+++ b/drivers/usb/phy/phy-samsung-usb2.c
@@ -284,8 +284,8 @@ static int samsung_usb2phy_init(struct usb_phy *phy)
/* Disable phy isolation */
if (sphy-plat  sphy-plat-pmu_isolation)
sphy-plat-pmu_isolation(false);
-   else
-   samsung_usbphy_set_isolation(sphy, false);
+   else if (sphy-drv_data-set_isolation)
+   sphy-drv_data-set_isolation(sphy, false);
 
/* Selecting Host/OTG mode; After reset USB2.0PHY_CFG: HOST */
samsung_usbphy_cfg_sel(sphy);
@@ -342,8 +342,8 @@ static void samsung_usb2phy_shutdown(struct usb_phy *phy)
/* Enable phy isolation */
if (sphy-plat  sphy-plat-pmu_isolation)

[PATCH v2 3/6] usb: phy: samsung: Consolidate reference clock rate handling

2013-05-16 Thread Tomasz Figa
This patch cleans up handling of reference clock rate in Samsung USB PHY
drivers. It is mostly a cosmetic change but improves error handling in
case of failing to get reference clock or invalid clock rate.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/phy/phy-samsung-usb.c  | 114 ++---
 drivers/usb/phy/phy-samsung-usb.h  |   7 +++
 drivers/usb/phy/phy-samsung-usb2.c |   8 ++-
 drivers/usb/phy/phy-samsung-usb3.c |   6 +-
 4 files changed, 86 insertions(+), 49 deletions(-)

diff --git a/drivers/usb/phy/phy-samsung-usb.c 
b/drivers/usb/phy/phy-samsung-usb.c
index 62cdb7e..c40ea32 100644
--- a/drivers/usb/phy/phy-samsung-usb.c
+++ b/drivers/usb/phy/phy-samsung-usb.c
@@ -162,13 +162,76 @@ int samsung_usbphy_set_type(struct usb_phy *phy,
 }
 EXPORT_SYMBOL_GPL(samsung_usbphy_set_type);
 
+int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy,
+   unsigned long rate)
+{
+   unsigned int clksel;
+
+   switch (rate) {
+   case 12 * MHZ:
+   clksel = PHYCLK_CLKSEL_12M;
+   break;
+   case 24 * MHZ:
+   clksel = PHYCLK_CLKSEL_24M;
+   break;
+   case 48 * MHZ:
+   clksel = PHYCLK_CLKSEL_48M;
+   break;
+   default:
+   dev_err(sphy-dev,
+   Invalid reference clock frequency: %lu\n, rate);
+   return -EINVAL;
+   }
+
+   return clksel;
+}
+EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx);
+
+int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy,
+   unsigned long rate)
+{
+   unsigned int clksel;
+
+   switch (rate) {
+   case 9600 * KHZ:
+   clksel = FSEL_CLKSEL_9600K;
+   break;
+   case 10 * MHZ:
+   clksel = FSEL_CLKSEL_10M;
+   break;
+   case 12 * MHZ:
+   clksel = FSEL_CLKSEL_12M;
+   break;
+   case 19200 * KHZ:
+   clksel = FSEL_CLKSEL_19200K;
+   break;
+   case 20 * MHZ:
+   clksel = FSEL_CLKSEL_20M;
+   break;
+   case 24 * MHZ:
+   clksel = FSEL_CLKSEL_24M;
+   break;
+   case 50 * MHZ:
+   clksel = FSEL_CLKSEL_50M;
+   break;
+   default:
+   dev_err(sphy-dev,
+   Invalid reference clock frequency: %lu\n, rate);
+   return -EINVAL;
+   }
+
+   return clksel;
+}
+EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_4x12);
+
 /*
  * Returns reference clock frequency selection value
  */
 int samsung_usbphy_get_refclk_freq(struct samsung_usbphy *sphy)
 {
struct clk *ref_clk;
-   int refclk_freq = 0;
+   unsigned long rate;
+   int refclk_freq;
 
/*
 * In exynos5250 USB host and device PHY use
@@ -183,52 +246,9 @@ int samsung_usbphy_get_refclk_freq(struct samsung_usbphy 
*sphy)
return PTR_ERR(ref_clk);
}
 
-   if (sphy-drv_data-cpu_type == TYPE_EXYNOS5250) {
-   /* set clock frequency for PLL */
-   switch (clk_get_rate(ref_clk)) {
-   case 9600 * KHZ:
-   refclk_freq = FSEL_CLKSEL_9600K;
-   break;
-   case 10 * MHZ:
-   refclk_freq = FSEL_CLKSEL_10M;
-   break;
-   case 12 * MHZ:
-   refclk_freq = FSEL_CLKSEL_12M;
-   break;
-   case 19200 * KHZ:
-   refclk_freq = FSEL_CLKSEL_19200K;
-   break;
-   case 20 * MHZ:
-   refclk_freq = FSEL_CLKSEL_20M;
-   break;
-   case 50 * MHZ:
-   refclk_freq = FSEL_CLKSEL_50M;
-   break;
-   case 24 * MHZ:
-   default:
-   /* default reference clock */
-   refclk_freq = FSEL_CLKSEL_24M;
-   break;
-   }
-   } else {
-   switch (clk_get_rate(ref_clk)) {
-   case 12 * MHZ:
-   refclk_freq = PHYCLK_CLKSEL_12M;
-   break;
-   case 24 * MHZ:
-   refclk_freq = PHYCLK_CLKSEL_24M;
-   break;
-   case 48 * MHZ:
-   refclk_freq = PHYCLK_CLKSEL_48M;
-   break;
-   default:
-   if (sphy-drv_data-cpu_type == TYPE_S3C64XX)
-   refclk_freq = PHYCLK_CLKSEL_48M;
-   else
-   refclk_freq = PHYCLK_CLKSEL_24M;
-   break;
-   }
-   }
+   rate = 

[PATCH v2 5/6] usb: phy: samsung: Pass enable/disable callbacks through driver data

2013-05-16 Thread Tomasz Figa
To remove unnecessary if statements, this patch introduces phy_enable
and phy_disable callbacks in driver data structure that implement
SoC-specific PHY initialization and deinitialization.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/phy/phy-samsung-usb.h  |  2 ++
 drivers/usb/phy/phy-samsung-usb2.c | 16 
 drivers/usb/phy/phy-samsung-usb3.c | 10 +-
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/phy/phy-samsung-usb.h 
b/drivers/usb/phy/phy-samsung-usb.h
index 5203784..31e2ec3 100644
--- a/drivers/usb/phy/phy-samsung-usb.h
+++ b/drivers/usb/phy/phy-samsung-usb.h
@@ -272,6 +272,8 @@ struct samsung_usbphy_drvdata {
u32 hostphy_reg_offset;
int (*rate_to_clksel)(struct samsung_usbphy *, unsigned long);
void (*set_isolation)(struct samsung_usbphy *, bool);
+   void (*phy_enable)(struct samsung_usbphy *);
+   void (*phy_disable)(struct samsung_usbphy *);
 };
 
 /*
diff --git a/drivers/usb/phy/phy-samsung-usb2.c 
b/drivers/usb/phy/phy-samsung-usb2.c
index ae6da68..b81347b 100644
--- a/drivers/usb/phy/phy-samsung-usb2.c
+++ b/drivers/usb/phy/phy-samsung-usb2.c
@@ -291,10 +291,7 @@ static int samsung_usb2phy_init(struct usb_phy *phy)
samsung_usbphy_cfg_sel(sphy);
 
/* Initialize usb phy registers */
-   if (sphy-drv_data-cpu_type == TYPE_EXYNOS5250)
-   samsung_exynos5_usb2phy_enable(sphy);
-   else
-   samsung_usb2phy_enable(sphy);
+   sphy-drv_data-phy_enable(sphy);
 
spin_unlock_irqrestore(sphy-lock, flags);
 
@@ -334,10 +331,7 @@ static void samsung_usb2phy_shutdown(struct usb_phy *phy)
}
 
/* De-initialize usb phy registers */
-   if (sphy-drv_data-cpu_type == TYPE_EXYNOS5250)
-   samsung_exynos5_usb2phy_disable(sphy);
-   else
-   samsung_usb2phy_disable(sphy);
+   sphy-drv_data-phy_disable(sphy);
 
/* Enable phy isolation */
if (sphy-plat  sphy-plat-pmu_isolation)
@@ -448,6 +442,8 @@ static const struct samsung_usbphy_drvdata usb2phy_s3c64xx 
= {
.devphy_en_mask = S3C64XX_USBPHY_ENABLE,
.rate_to_clksel = samsung_usbphy_rate_to_clksel_64xx,
.set_isolation  = NULL, /* TODO */
+   .phy_enable = samsung_usb2phy_enable,
+   .phy_disable= samsung_usb2phy_disable,
 };
 
 static const struct samsung_usbphy_drvdata usb2phy_exynos4 = {
@@ -456,6 +452,8 @@ static const struct samsung_usbphy_drvdata usb2phy_exynos4 
= {
.hostphy_en_mask= EXYNOS_USBPHY_ENABLE,
.rate_to_clksel = samsung_usbphy_rate_to_clksel_64xx,
.set_isolation  = samsung_usbphy_set_isolation_4210,
+   .phy_enable = samsung_usb2phy_enable,
+   .phy_disable= samsung_usb2phy_disable,
 };
 
 static struct samsung_usbphy_drvdata usb2phy_exynos5 = {
@@ -464,6 +462,8 @@ static struct samsung_usbphy_drvdata usb2phy_exynos5 = {
.hostphy_reg_offset = EXYNOS_USBHOST_PHY_CTRL_OFFSET,
.rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,
.set_isolation  = samsung_usbphy_set_isolation_4210,
+   .phy_enable = samsung_exynos5_usb2phy_enable,
+   .phy_disable= samsung_exynos5_usb2phy_disable,
 };
 
 #ifdef CONFIG_OF
diff --git a/drivers/usb/phy/phy-samsung-usb3.c 
b/drivers/usb/phy/phy-samsung-usb3.c
index ae232ad..b663353 100644
--- a/drivers/usb/phy/phy-samsung-usb3.c
+++ b/drivers/usb/phy/phy-samsung-usb3.c
@@ -65,7 +65,7 @@ static u32 samsung_usb3phy_set_refclk(struct samsung_usbphy 
*sphy)
return reg;
 }
 
-static int samsung_exynos5_usb3phy_enable(struct samsung_usbphy *sphy)
+static void samsung_exynos5_usb3phy_enable(struct samsung_usbphy *sphy)
 {
void __iomem *regs = sphy-regs;
u32 phyparam0;
@@ -133,8 +133,6 @@ static int samsung_exynos5_usb3phy_enable(struct 
samsung_usbphy *sphy)
 
phyclkrst = ~(PHYCLKRST_PORTRESET);
writel(phyclkrst, regs + EXYNOS5_DRD_PHYCLKRST);
-
-   return 0;
 }
 
 static void samsung_exynos5_usb3phy_disable(struct samsung_usbphy *sphy)
@@ -188,7 +186,7 @@ static int samsung_usb3phy_init(struct usb_phy *phy)
sphy-drv_data-set_isolation(sphy, false);
 
/* Initialize usb phy registers */
-   samsung_exynos5_usb3phy_enable(sphy);
+   sphy-drv_data-phy_enable(sphy);
 
spin_unlock_irqrestore(sphy-lock, flags);
 
@@ -219,7 +217,7 @@ static void samsung_usb3phy_shutdown(struct usb_phy *phy)
samsung_usbphy_set_type(sphy-phy, USB_PHY_TYPE_DEVICE);
 
/* De-initialize usb phy registers */
-   samsung_exynos5_usb3phy_disable(sphy);
+   sphy-drv_data-phy_disable(sphy);
 
/* Enable phy isolation */
if (sphy-drv_data-set_isolation)
@@ -312,6 +310,8 @@ static struct samsung_usbphy_drvdata usb3phy_exynos5 = {
   

[PATCH v2 6/6] usb: phy: samsung: Add support for USB 2.0 PHY on Exynos 4x12

2013-05-16 Thread Tomasz Figa
This patch adds driver data for Exynos 4x12 USB 2.0 PHY.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/phy/phy-samsung-usb.h  |  1 +
 drivers/usb/phy/phy-samsung-usb2.c | 18 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/phy/phy-samsung-usb.h 
b/drivers/usb/phy/phy-samsung-usb.h
index 31e2ec3..585d12f 100644
--- a/drivers/usb/phy/phy-samsung-usb.h
+++ b/drivers/usb/phy/phy-samsung-usb.h
@@ -241,6 +241,7 @@
 enum samsung_cpu_type {
TYPE_S3C64XX,
TYPE_EXYNOS4210,
+   TYPE_EXYNOS4X12,
TYPE_EXYNOS5250,
 };
 
diff --git a/drivers/usb/phy/phy-samsung-usb2.c 
b/drivers/usb/phy/phy-samsung-usb2.c
index b81347b..381f8d4 100644
--- a/drivers/usb/phy/phy-samsung-usb2.c
+++ b/drivers/usb/phy/phy-samsung-usb2.c
@@ -177,6 +177,7 @@ static void samsung_usb2phy_enable(struct samsung_usbphy 
*sphy)
rstcon |= RSTCON_SWRST;
break;
case TYPE_EXYNOS4210:
+   case TYPE_EXYNOS4X12:
phypwr = ~PHYPWR_NORMAL_MASK_PHY0;
rstcon |= RSTCON_SWRST;
default:
@@ -240,6 +241,7 @@ static void samsung_usb2phy_disable(struct samsung_usbphy 
*sphy)
phypwr |= PHYPWR_NORMAL_MASK;
break;
case TYPE_EXYNOS4210:
+   case TYPE_EXYNOS4X12:
phypwr |= PHYPWR_NORMAL_MASK_PHY0;
default:
break;
@@ -456,6 +458,16 @@ static const struct samsung_usbphy_drvdata usb2phy_exynos4 
= {
.phy_disable= samsung_usb2phy_disable,
 };
 
+static const struct samsung_usbphy_drvdata usb2phy_exynos4x12 = {
+   .cpu_type   = TYPE_EXYNOS4X12,
+   .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
+   .hostphy_en_mask= EXYNOS_USBPHY_ENABLE,
+   .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,
+   .set_isolation  = samsung_usbphy_set_isolation_4210,
+   .phy_enable = samsung_usb2phy_enable,
+   .phy_disable= samsung_usb2phy_disable,
+};
+
 static struct samsung_usbphy_drvdata usb2phy_exynos5 = {
.cpu_type   = TYPE_EXYNOS5250,
.hostphy_en_mask= EXYNOS_USBPHY_ENABLE,
@@ -475,6 +487,9 @@ static const struct of_device_id samsung_usbphy_dt_match[] 
= {
.compatible = samsung,exynos4210-usb2phy,
.data = usb2phy_exynos4,
}, {
+   .compatible = samsung,exynos4x12-usb2phy,
+   .data = usb2phy_exynos4x12,
+   }, {
.compatible = samsung,exynos5250-usb2phy,
.data = usb2phy_exynos5
},
@@ -491,6 +506,9 @@ static struct platform_device_id 
samsung_usbphy_driver_ids[] = {
.name   = exynos4210-usb2phy,
.driver_data= (unsigned long)usb2phy_exynos4,
}, {
+   .name   = exynos4x12-usb2phy,
+   .driver_data= (unsigned long)usb2phy_exynos4x12,
+   }, {
.name   = exynos5250-usb2phy,
.driver_data= (unsigned long)usb2phy_exynos5,
},
-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 30/33] arch/arm/plat-samsung: don't check resource with devm_ioremap_resource

2013-05-16 Thread Wolfram Sang
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 arch/arm/plat-samsung/adc.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/plat-samsung/adc.c b/arch/arm/plat-samsung/adc.c
index ca07cb1..79690f2 100644
--- a/arch/arm/plat-samsung/adc.c
+++ b/arch/arm/plat-samsung/adc.c
@@ -381,11 +381,6 @@ static int s3c_adc_probe(struct platform_device *pdev)
}
 
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!regs) {
-   dev_err(dev, failed to find registers\n);
-   return -ENXIO;
-   }
-
adc-regs = devm_ioremap_resource(dev, regs);
if (IS_ERR(adc-regs))
return PTR_ERR(adc-regs);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/33] drivers/i2c/busses: don't check resource with devm_ioremap_resource

2013-05-16 Thread Wolfram Sang
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/i2c/busses/i2c-s3c2410.c |5 -
 drivers/i2c/busses/i2c-sirf.c|6 --
 drivers/i2c/busses/i2c-tegra.c   |5 -
 3 files changed, 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 6e8ee92..cab1c91 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1082,11 +1082,6 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
/* map the registers */
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (res == NULL) {
-   dev_err(pdev-dev, cannot find IO resource\n);
-   return -ENOENT;
-   }
-
i2c-regs = devm_ioremap_resource(pdev-dev, res);
 
if (IS_ERR(i2c-regs))
diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
index 5a7ad24..a63c7d5 100644
--- a/drivers/i2c/busses/i2c-sirf.c
+++ b/drivers/i2c/busses/i2c-sirf.c
@@ -303,12 +303,6 @@ static int i2c_sirfsoc_probe(struct platform_device *pdev)
adap-class = I2C_CLASS_HWMON;
 
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (mem_res == NULL) {
-   dev_err(pdev-dev, Unable to get MEM resource\n);
-   err = -EINVAL;
-   goto out;
-   }
-
siic-base = devm_ioremap_resource(pdev-dev, mem_res);
if (IS_ERR(siic-base)) {
err = PTR_ERR(siic-base);
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index b60ff90..9aa1b60 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -714,11 +714,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
int ret = 0;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(pdev-dev, no mem resource\n);
-   return -EINVAL;
-   }
-
base = devm_ioremap_resource(pdev-dev, res);
if (IS_ERR(base))
return PTR_ERR(base);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/33] drivers/i2c/busses: don't check resource with devm_ioremap_resource

2013-05-16 Thread Barry Song
2013/5/16 Wolfram Sang w...@the-dreams.de:
 devm_ioremap_resource does sanity checks on the given resource. No need to
 duplicate this in the driver.

 Signed-off-by: Wolfram Sang w...@the-dreams.de

Acked-by: Barry Song baohua.s...@csr.com

-barry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] Fix suspend/resume issues created by pinmux on exynos

2013-05-16 Thread Doug Anderson

This set of patches fixes some problems with suspend/resume that were
introduced by the switch from the old gpio code to the new pinmux
code.  Specifically:
* It adds saving and restoring of pincontrol registers.
* It fixes eint wakeups.

This set of two patches was verified on a backport of the current
pinmux code onto 3.8 on a Samsung ARM Chromebook.  Suspend/resume does
not seem functional on the ARM Chromebook on current ToT Linux so I
couldn't validate there.  This gets us one step closer, though!  Since
patches applied cleanly I'm fairly certain that they will work on ToT
as well as they do in our tree.

These patches have only been tested on exynos5250.  I have made an
effort to support other samsung boards (even those with two CONF
registers), but that support is untested.

Tomasz Figa has said that he has similar patches in development.  I'm
posting what we have here but if Tomasz's patches end up being more
suitable I have no objections to taking them over these (or of Tomasz
wants to merge the two?).

If you'd like to see the gerrit reviews of these in the Chrome OS tree,
you can see:
* https://gerrit.chromium.org/gerrit/#/c/51336/4
* https://gerrit.chromium.org/gerrit/#/c/51342/3


Doug Anderson (1):
  pinctrl: samsung: fix suspend/resume functionality

Prathyush K (1):
  pinctrl: exynos: fix eint wakeup by using irq_set_wake()

 drivers/pinctrl/pinctrl-exynos.c  |  45 ++---
 drivers/pinctrl/pinctrl-exynos.h  |   3 +-
 drivers/pinctrl/pinctrl-samsung.c | 199 ++
 drivers/pinctrl/pinctrl-samsung.h |  13 +++
 4 files changed, 247 insertions(+), 13 deletions(-)

-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
The GPIO states need to be restored after s2r and this is not currently
supported in the pinctrl driver. This patch saves the gpio states before
suspend and restores them after resume.

Logic and commenting for samsung_pinctrl_resume_noirq() is heavily
borrowed from the old samsung_gpio_pm_2bit_resume(), which seemed to
do this a reasonable way.

Patch originally from Prathyush K prathyus...@samsung.com but
rewritten by Doug Anderson diand...@chromium.org.

Signed-off-by: Prathyush K prathyus...@samsung.com
Signed-off-by: Doug Anderson diand...@chromium.org
---
 drivers/pinctrl/pinctrl-samsung.c | 199 ++
 drivers/pinctrl/pinctrl-samsung.h |  11 +++
 2 files changed, 210 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 9763668..0891667 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -964,6 +964,204 @@ static int samsung_pinctrl_probe(struct platform_device 
*pdev)
return 0;
 }
 
+#ifdef CONFIG_PM
+
+/**
+ * samsung_pinctrl_resume_noirq - save pinctrl state for suspend
+ *
+ * Save data for all banks handled by this device.
+ */
+static int samsung_pinctrl_suspend_noirq(struct device *dev)
+{
+   struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev);
+   struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
+   void __iomem * const virt_base = drvdata-virt_base;
+   int i;
+
+   for (i = 0; i  ctrl-nr_banks; i++) {
+   struct samsung_pin_bank *bank = ctrl-pin_banks[i];
+   const struct samsung_pin_bank_type *type = bank-type;
+   void __iomem * const reg = virt_base + bank-pctl_offset;
+
+   bank-pm_save.con = readl(reg +
+ type-reg_offset[PINCFG_TYPE_FUNC]);
+   if (type-fld_width[PINCFG_TYPE_FUNC]  4)
+   bank-pm_save.con |= (u64)readl(reg + 4 +
+ type-reg_offset[PINCFG_TYPE_FUNC])  32;
+   bank-pm_save.dat = readl(reg +
+ type-reg_offset[PINCFG_TYPE_DAT]);
+   bank-pm_save.pud = readl(reg +
+ type-reg_offset[PINCFG_TYPE_PUD]);
+   bank-pm_save.drv = readl(reg +
+ type-reg_offset[PINCFG_TYPE_DRV]);
+
+   if (type-fld_width[PINCFG_TYPE_CON_PDN]) {
+   bank-pm_save.conpdn = readl(reg +
+   type-reg_offset[PINCFG_TYPE_CON_PDN]);
+   bank-pm_save.pudpdn = readl(reg +
+   type-reg_offset[PINCFG_TYPE_PUD_PDN]);
+   }
+
+   dev_dbg(dev, Save %s @ %p (con %#010llx)\n,
+   bank-name, reg, bank-pm_save.con);
+   }
+
+   return 0;
+}
+
+/**
+ * is_sfn - test whether a pin config represents special function.
+ *
+ * Test whether the given masked+shifted bits of an GPIO configuration
+ * are one of the SFN (special function) modes.
+ */
+static inline int is_sfn(u32 con)
+{
+   return con = 2;
+}
+
+/**
+ * is_in - test if the given masked+shifted GPIO configuration is an input.
+ */
+static inline int is_in(u32 con)
+{
+   return con == 0;
+}
+
+/**
+ * is_out - test if the given masked+shifted GPIO configuration is an output.
+ */
+static inline int is_out(u32 con)
+{
+   return con == 1;
+}
+
+/**
+ * samsung_pinctrl_resume_noirq - restore pinctrl state from suspend
+ *
+ * Restore one of the GPIO banks that was saved during suspend. This is
+ * not as simple as once thought, due to the possibility of glitches
+ * from the order that the CON and DAT registers are set in.
+ *
+ * The three states the pin can be are {IN,OUT,SFN} which gives us 9
+ * combinations of changes to check. Three of these, if the pin stays
+ * in the same configuration can be discounted. This leaves us with
+ * the following:
+ *
+ * { IN = OUT }  Change DAT first
+ * { IN = SFN }  Change CON first
+ * { OUT = SFN } Change CON first, so new data will not glitch
+ * { OUT = IN }  Change CON first, so new data will not glitch
+ * { SFN = IN }  Change CON first
+ * { SFN = OUT } Change DAT first, so new data will not glitch [1]
+ *
+ * We do not currently deal with the UP registers as these control
+ * weak resistors, so a small delay in change should not need to bring
+ * these into the calculations.
+ *
+ * [1] this assumes that writing to a pin DAT whilst in SFN will set the
+ * state for when it is next output.
+ */
+static int samsung_pinctrl_resume_noirq(struct device *dev)
+{
+   struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev);
+   struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
+   void __iomem * const virt_base = drvdata-virt_base;
+   int i;
+
+   for (i = 0; i  ctrl-nr_banks; i++) {
+   const struct samsung_pin_bank *bank = ctrl-pin_banks[i];
+  

[PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()

2013-05-16 Thread Doug Anderson
From: Prathyush K prathyus...@samsung.com

Add the irq_set_wake function for exynos pinctrl to configure the
external interrupt wakeup mask register.

[dianders: minor nit fixes; port to ToT]

Signed-off-by: Prathyush K prathyus...@samsung.com
Signed-off-by: Doug Anderson diand...@chromium.org
---
 drivers/pinctrl/pinctrl-exynos.c  | 45 ---
 drivers/pinctrl/pinctrl-exynos.h  |  3 ++-
 drivers/pinctrl/pinctrl-samsung.h |  2 ++
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index ac74281..3ebb2ff 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -30,6 +30,8 @@
 #include linux/spinlock.h
 #include linux/err.h
 
+#include plat/pm.h
+
 #include pinctrl-samsung.h
 #include pinctrl-exynos.h
 
@@ -326,6 +328,24 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, 
unsigned int type)
return 0;
 }
 
+static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int state)
+{
+   struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+   const int eint_num = bank-eint_base + irqd-hwirq;
+   unsigned long bit = 1L  eint_num;
+
+   pr_info(wake %s for eint %d / %s[%ld]\n,
+   state ? enabled : disabled,
+   eint_num, bank-name, irqd-hwirq);
+
+   if (!state)
+   s3c_irqwake_eintmask |= bit;
+   else
+   s3c_irqwake_eintmask = ~bit;
+
+   return 0;
+}
+
 /*
  * irq_chip for wakeup interrupts
  */
@@ -335,6 +355,7 @@ static struct irq_chip exynos_wkup_irq_chip = {
.irq_mask   = exynos_wkup_irq_mask,
.irq_ack= exynos_wkup_irq_ack,
.irq_set_type   = exynos_wkup_irq_set_type,
+   .irq_set_wake   = exynos_wkup_irq_set_wake,
 };
 
 /* interrupt handler for wakeup interrupts 0..15 */
@@ -543,10 +564,10 @@ static struct samsung_pin_bank exynos4210_pin_banks1[] = {
EXYNOS_PIN_BANK_EINTN(8, 0x1A0, gpy4),
EXYNOS_PIN_BANK_EINTN(8, 0x1C0, gpy5),
EXYNOS_PIN_BANK_EINTN(8, 0x1E0, gpy6),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24),
 };
 
 /* pin banks of exynos4210 pin-controller 2 */
@@ -629,10 +650,10 @@ static struct samsung_pin_bank exynos4x12_pin_banks1[] = {
EXYNOS_PIN_BANK_EINTN(8, 0x1A0, gpy4),
EXYNOS_PIN_BANK_EINTN(8, 0x1C0, gpy5),
EXYNOS_PIN_BANK_EINTN(8, 0x1E0, gpy6),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24),
 };
 
 /* pin banks of exynos4x12 pin-controller 2 */
@@ -724,10 +745,10 @@ static struct samsung_pin_bank exynos5250_pin_banks0[] = {
EXYNOS_PIN_BANK_EINTN(8, 0x220, gpy4),
EXYNOS_PIN_BANK_EINTN(8, 0x240, gpy5),
EXYNOS_PIN_BANK_EINTN(8, 0x260, gpy6),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08),
-   EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16),
+   EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24),
 };
 
 /* pin banks of exynos5250 pin-controller 1 */
diff --git a/drivers/pinctrl/pinctrl-exynos.h b/drivers/pinctrl/pinctrl-exynos.h
index 9b1f77a..d98e9ff 100644
--- a/drivers/pinctrl/pinctrl-exynos.h
+++ b/drivers/pinctrl/pinctrl-exynos.h
@@ -65,13 +65,14 @@
.name   = id\
}
 
-#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \
+#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\
{   \
.type   = bank_type_alive, \
.pctl_offset= reg,  \
.nr_pins= pins, \
.eint_type  = EINT_TYPE_WKUP,   \
.eint_offset= offs, \
+   .eint_base  = base, \
.name   = id\
}
 
diff --git a/drivers/pinctrl/pinctrl-samsung.h 
b/drivers/pinctrl/pinctrl-samsung.h

Re: Pulls and drive strengths in the pinctrl world

2013-05-16 Thread Doug Anderson
Tomasz / Stephen,

On Wed, May 15, 2013 at 5:55 PM, Doug Anderson diand...@google.com wrote:
 Also after reading Stephen's reply, I'm wondering if hogging wouldn't
 solve the problem indeed. (It might have to be fixed on pinctrl-samsung
 first, as last time I tried to use it, it caused some errors from pinctrl
 core, but haven't time to track them down, as it wasn't anything important
 at that time).

 I will give it a shot tomorrow morning and see how it looks.  I'll
 also evaluate Stephen's suggestions then once I've see how it looks
 with the current bindings...

I wrote this out and it had some nice properties (it was a little more
concise), but had the downside that there's no reference from the GPIO
usage back to the pinmux.  I'd rather have the reference there in the
hopes that it will help others get things right when they make changes
to the dts file (they'll notice the reference and know that they need
to change that too).

...so I think the summary is: I'm OK with keeping what we have.  It
may be a little awkward in some ways but it's definitely worth it to
get all of the benefits of the pinmux / GPIO separation.  :)

For the curious of what my prototype looked like (feel free to
ignore--I'm not planning on keeping this and I didn't actually try
testing it), I've included it below.  This is just the bit from
cros5250-common (the common file shared among several similar
boards), so I'd need something similar in exynos5250-snow).

pinctrl@1140 {
  /* Default states for hogs follow */
  nopull_inputs_cros5250_a: nopull-inputs-cros5250-a {
samsung,pins =
  gpx1-2, /* trackpad */
  gpx1-3, /* gpio-keys - power */
  gpx3-2; /* max77686 */
samsung,pin-function = 0;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
  };

  pulldown_inputs_cros5250-a: pulldown-inputs-cros5250_a {
samsung,pins = gpx3-7; /* hdmi */
samsung,pin-function = 0;
samsung,pin-pud = 1;
samsung,pin-drv = 0;
  };

  simple_outputs_cros5250-a: simple-outputs-cros5250_a {
samsung,pins =
  gpx0-1, /* wifi-en */
  gpx0-2, /* wifi-rst */
  gpx1-7; /* max98095-en */
samsung,pin-function = 1;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
  }

  pinctrl-names = default;
  pinctrl-0 = nopull_inputs_cros5250_a
   pulldown_inputs_cros5250_a
   simple_outputs_cros5250_a;
};

pinctrl@1340 {
  simple_outputs_cros5250-b: simple-outputs-cros5250_b {
samsung,pins = gpe1-0 /* hsic reset */;
samsung,pin-function = 1;
samsung,pin-pud = 0;
samsung,pin-drv = 0;

  };
  pinctrl-names = default;
  pinctrl-0 = simple_outputs_cros5250_b;
};

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Tomasz Figa
Hi Doug,

Thanks for the patch. See my comments inline.

On Thursday 16 of May 2013 10:12:31 Doug Anderson wrote:
 The GPIO states need to be restored after s2r and this is not currently
 supported in the pinctrl driver. This patch saves the gpio states before
 suspend and restores them after resume.
 
 Logic and commenting for samsung_pinctrl_resume_noirq() is heavily
 borrowed from the old samsung_gpio_pm_2bit_resume(), which seemed to
 do this a reasonable way.
 
 Patch originally from Prathyush K prathyus...@samsung.com but
 rewritten by Doug Anderson diand...@chromium.org.
 
 Signed-off-by: Prathyush K prathyus...@samsung.com
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/pinctrl/pinctrl-samsung.c | 199
 ++
 drivers/pinctrl/pinctrl-samsung.h |  11 +++
  2 files changed, 210 insertions(+)
 
 diff --git a/drivers/pinctrl/pinctrl-samsung.c
 b/drivers/pinctrl/pinctrl-samsung.c index 9763668..0891667 100644
 --- a/drivers/pinctrl/pinctrl-samsung.c
 +++ b/drivers/pinctrl/pinctrl-samsung.c
 @@ -964,6 +964,204 @@ static int samsung_pinctrl_probe(struct
 platform_device *pdev) return 0;
  }
 
 +#ifdef CONFIG_PM
 +
 +/**
 + * samsung_pinctrl_resume_noirq - save pinctrl state for suspend
 + *
 + * Save data for all banks handled by this device.
 + */
 +static int samsung_pinctrl_suspend_noirq(struct device *dev)
 +{
 + struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev);
 + struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
 + void __iomem * const virt_base = drvdata-virt_base;

Nit: This const is ugly :) . Is it needed for anything?

 + int i;
 +
 + for (i = 0; i  ctrl-nr_banks; i++) {
 + struct samsung_pin_bank *bank = ctrl-pin_banks[i];
 + const struct samsung_pin_bank_type *type = bank-type;
 + void __iomem * const reg = virt_base + bank-pctl_offset;

Nit: This one is not pretty either.

 + bank-pm_save.con = readl(reg +
 +   type-
reg_offset[PINCFG_TYPE_FUNC]);
 + if (type-fld_width[PINCFG_TYPE_FUNC]  4)

What is this condition supposed to check?

 + bank-pm_save.con |= (u64)readl(reg + 4 +
 +   type-reg_offset[PINCFG_TYPE_FUNC]) 
 32;

This looks ugly. Whatever is going on here, wouldn't it be better to use 
separate field, like con2 or something?

 + bank-pm_save.dat = readl(reg +
 +   type-
reg_offset[PINCFG_TYPE_DAT]);
 + bank-pm_save.pud = readl(reg +
 +   type-
reg_offset[PINCFG_TYPE_PUD]);
 + bank-pm_save.drv = readl(reg +
 +   type-
reg_offset[PINCFG_TYPE_DRV]);
 +
 + if (type-fld_width[PINCFG_TYPE_CON_PDN]) {
 + bank-pm_save.conpdn = readl(reg +
 + type-reg_offset[PINCFG_TYPE_CON_PDN]);
 + bank-pm_save.pudpdn = readl(reg +
 + type-reg_offset[PINCFG_TYPE_PUD_PDN]);
 + }

I wonder if you couldn't do all the saving here in a single loop over all 
pin control types, like:

unsigned int offsets = bank-type-reg_offsets;
unsigned int widths = bank-type-fld_width;

for (i = 0; i  PINCFG_TYPE_NUM; ++i)
if (widths[i])
bank-pm_save[i] = readl(reg + offsets[i]);

The only thing not handled by this loop is second CON registers in banks 
with two of them. I can't think of any better solution for this other than 
just adding a special case after the loop.

 + dev_dbg(dev, Save %s @ %p (con %#010llx)\n,
 + bank-name, reg, bank-pm_save.con);
 + }
 +
 + return 0;
 +}
 +
 +/**
 + * is_sfn - test whether a pin config represents special function.
 + *
 + * Test whether the given masked+shifted bits of an GPIO configuration
 + * are one of the SFN (special function) modes.
 + */
 +static inline int is_sfn(u32 con)
 +{
 + return con = 2;
 +}
 +
 +/**
 + * is_in - test if the given masked+shifted GPIO configuration is an
 input. + */
 +static inline int is_in(u32 con)
 +{
 + return con == 0;
 +}
 +
 +/**
 + * is_out - test if the given masked+shifted GPIO configuration is an
 output. + */
 +static inline int is_out(u32 con)
 +{
 + return con == 1;
 +}
 +
 +/**
 + * samsung_pinctrl_resume_noirq - restore pinctrl state from suspend
 + *
 + * Restore one of the GPIO banks that was saved during suspend. This is
 + * not as simple as once thought, due to the possibility of glitches +
 * from the order that the CON and DAT registers are set in.
 + *
 + * The three states the pin can be are {IN,OUT,SFN} which gives us 9
 + * combinations of changes to check. Three of these, if the pin stays
 + * in the same configuration can be discounted. This leaves us with
 + * the following:
 + *
 + * { IN = OUT }  Change DAT first
 + * { IN 

Re: [PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()

2013-05-16 Thread Tomasz Figa
On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote:
 From: Prathyush K prathyus...@samsung.com
 
 Add the irq_set_wake function for exynos pinctrl to configure the
 external interrupt wakeup mask register.
 
 [dianders: minor nit fixes; port to ToT]
 
 Signed-off-by: Prathyush K prathyus...@samsung.com
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/pinctrl/pinctrl-exynos.c  | 45
 ---
 drivers/pinctrl/pinctrl-exynos.h  |  3 ++-
  drivers/pinctrl/pinctrl-samsung.h |  2 ++
  3 files changed, 37 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/pinctrl/pinctrl-exynos.c
 b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644
 --- a/drivers/pinctrl/pinctrl-exynos.c
 +++ b/drivers/pinctrl/pinctrl-exynos.c
 @@ -30,6 +30,8 @@
  #include linux/spinlock.h
  #include linux/err.h
 
 +#include plat/pm.h
 +

This is not going to work with CONFIG_MULTIPLATFORM.

Now this raises a question what is the preferred way to pass some data 
from generic driver to platform code.

I would suggest adding a function called exynos_pinctrl_get_eintmask() (or 
whatever) that would return the wake-up mask configured in the driver and 
then modify platform code to use it.

  #include pinctrl-samsung.h
  #include pinctrl-exynos.h
 
 @@ -326,6 +328,24 @@ static int exynos_wkup_irq_set_type(struct irq_data
 *irqd, unsigned int type) return 0;
  }
 
 +static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int
 state) +{
 + struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
 + const int eint_num = bank-eint_base + irqd-hwirq;
 + unsigned long bit = 1L  eint_num;
 +
 + pr_info(wake %s for eint %d / %s[%ld]\n,
 + state ? enabled : disabled,
 + eint_num, bank-name, irqd-hwirq);
 +
 + if (!state)
 + s3c_irqwake_eintmask |= bit;
 + else
 + s3c_irqwake_eintmask = ~bit;
 +
 + return 0;
 +}
 +
  /*
   * irq_chip for wakeup interrupts
   */
 @@ -335,6 +355,7 @@ static struct irq_chip exynos_wkup_irq_chip = {
   .irq_mask   = exynos_wkup_irq_mask,
   .irq_ack= exynos_wkup_irq_ack,
   .irq_set_type   = exynos_wkup_irq_set_type,
 + .irq_set_wake   = exynos_wkup_irq_set_wake,
  };
 
  /* interrupt handler for wakeup interrupts 0..15 */
 @@ -543,10 +564,10 @@ static struct samsung_pin_bank
 exynos4210_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, gpy4),
   EXYNOS_PIN_BANK_EINTN(8, 0x1C0, gpy5),
   EXYNOS_PIN_BANK_EINTN(8, 0x1E0, gpy6),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24),
  };
 
  /* pin banks of exynos4210 pin-controller 2 */
 @@ -629,10 +650,10 @@ static struct samsung_pin_bank
 exynos4x12_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, gpy4),
   EXYNOS_PIN_BANK_EINTN(8, 0x1C0, gpy5),
   EXYNOS_PIN_BANK_EINTN(8, 0x1E0, gpy6),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24),
  };
 
  /* pin banks of exynos4x12 pin-controller 2 */
 @@ -724,10 +745,10 @@ static struct samsung_pin_bank
 exynos5250_pin_banks0[] = { EXYNOS_PIN_BANK_EINTN(8, 0x220, gpy4),
   EXYNOS_PIN_BANK_EINTN(8, 0x240, gpy5),
   EXYNOS_PIN_BANK_EINTN(8, 0x260, gpy6),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08),
 - EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16),
 + EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24),
  };
 
  /* pin banks of exynos5250 pin-controller 1 */
 diff --git a/drivers/pinctrl/pinctrl-exynos.h
 b/drivers/pinctrl/pinctrl-exynos.h index 9b1f77a..d98e9ff 100644
 --- a/drivers/pinctrl/pinctrl-exynos.h
 +++ b/drivers/pinctrl/pinctrl-exynos.h
 @@ -65,13 +65,14 @@
   .name   = id\
   }
 
 -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs)   \
 +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\
   {   \
   .type   = bank_type_alive, \
   .pctl_offset= reg,  

Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
Tomasz,

Thanks for the review!  I'll get a new patch out either today or tomorrow...

On Thu, May 16, 2013 at 12:19 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 +/**
 + * samsung_pinctrl_resume_noirq - save pinctrl state for suspend
 + *
 + * Save data for all banks handled by this device.
 + */
 +static int samsung_pinctrl_suspend_noirq(struct device *dev)
 +{
 + struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev);
 + struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
 + void __iomem * const virt_base = drvdata-virt_base;

 Nit: This const is ugly :) . Is it needed for anything?

Not anything really.  I just got in the habit of adding them for
variables that are simple shorthand variables: AKA I'm only creating
this variable to avoid typing some long complicated thing below.  It's
a hint to someone reading the code that they don't need to think about
it.  I have also occasionally caught bugs by doing this.

...but I can understand the dislike.  I'll remove this and other
similar (but keep const pointers).


 + if (type-fld_width[PINCFG_TYPE_FUNC]  4)

 What is this condition supposed to check?

It is supposed to be checking whether there are two CON registers in
use.  ...oh, but that's probably not the right way to do it now that I
think about it.  I need to check (bank-nr_pins *
type-fld_width[PINCFG_TYPE_FUNC]).

I will fix.


 + bank-pm_save.con |= (u64)readl(reg + 4 +
 +   type-reg_offset[PINCFG_TYPE_FUNC])
  32;

 This looks ugly. Whatever is going on here, wouldn't it be better to use
 separate field, like con2 or something?

Probably.  The resume code seemed cleaner with a 64-bit value, but I
think I could make it nearly as clean with two 32-bit ones by using a
helper function.


 I wonder if you couldn't do all the saving here in a single loop over all
 pin control types, like:

 unsigned int offsets = bank-type-reg_offsets;
 unsigned int widths = bank-type-fld_width;

 for (i = 0; i  PINCFG_TYPE_NUM; ++i)
 if (widths[i])
 bank-pm_save[i] = readl(reg + offsets[i]);

 The only thing not handled by this loop is second CON registers in banks
 with two of them. I can't think of any better solution for this other than
 just adding a special case after the loop.

Yes, that would work.  I think it wasn't possible when I first wrote
the code against an older code base that didn't have the arrays.  I
can give it a shot if it doesn't make restore too bad...


 Now as I think of it, do CON_PDN and PUD_PDN really need to be saved? I
 couldn't find in the documentation if they are preserved or lost in sleep
 mode. Do you have some information on this?

I remember it being important.  Running a test now.  Yes, it's
important on exynos5250.  As an example:

[   62.22] samsung-pinctrl 1140.pinctrl: Restore gpa2@f0048040
(0x=0x2212 ch 0x)
[   62.22] samsung-pinctrl 1140.pinctrl: Restore gpb0@f0048060
CON_PDN (0x=0x02aa)
[   62.22] samsung-pinctrl 1140.pinctrl: Restore gpb0@f0048060
PUD_PDN (0x=0x0155)


 I wonder if the whole restoration procedure couldn't be simplified. I
 don't remember my version being so complicated, but I don't have my patch
 on my screen at the moment, so I might be wrong on this.

I debated about this a bunch.  Perhaps I should just delete it.  I saw
that it was there in the old 2-bit code and it also seemed quite
reasonable, so I kept it.  Things seem to work OK without it, but most
things are pretty tolerant to their lines glitching (or even driving
high to low for a short period of time).

...but your question made me check again.

From previous experimentation I'm pretty certain that most pins on the
exynos are held in the powerdown state even during early bootup of the
SoC.  The hope is that they are released from powerdown _after_ the
GPIO init is called.  If not then we're already glitching somewhat as
we transition from powerdown state to default state before this
function finally gets called.

Looking at exynos, that's probably done in exynos_pm_resume(), maybe
by mucking with the pad retention options?

Oh, that probably means taht no_irq() is too late and that I need to
figure out how to get my code called earlier...  The
exynos_pm_resume() is called by syscore.


 +#else
 +#define samsung_pinctrl_suspend_noirqNULL
 +#define samsung_pinctrl_resume_noirq NULL
 +#endif
 +
 +static const struct dev_pm_ops samsung_pinctrl_dev_pm_ops = {
 + .suspend_noirq = samsung_pinctrl_suspend_noirq,
 + .resume_noirq = samsung_pinctrl_resume_noirq,
 +};

 I'm not sure if resume_noirq is really early enough. Some drivers might
 already need correct pin configuration in their resume_noirq callback.

 In my patch I have used syscore_ops to register very late suspend and very
 early resume callbacks for the whole pinctrl-samsung driver and a global
 list of registered pin 

Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Heiko Stübner
Am Donnerstag, 16. Mai 2013, 21:19:20 schrieb Tomasz Figa:
[...]
  +
  +   if (type-fld_width[PINCFG_TYPE_CON_PDN]) {
  +   bank-pm_save.conpdn = readl(reg +
  +   type-reg_offset[PINCFG_TYPE_CON_PDN]);
  +   bank-pm_save.pudpdn = readl(reg +
  +   type-reg_offset[PINCFG_TYPE_PUD_PDN]);
  +   }
 
 I wonder if you couldn't do all the saving here in a single loop over all
 pin control types, like:
 
   unsigned int offsets = bank-type-reg_offsets;
   unsigned int widths = bank-type-fld_width;
 
   for (i = 0; i  PINCFG_TYPE_NUM; ++i)
   if (widths[i])
   bank-pm_save[i] = readl(reg + offsets[i]);
 
 The only thing not handled by this loop is second CON registers in banks
 with two of them. I can't think of any better solution for this other than
 just adding a special case after the loop.

doing this in the loop over the pinctrl types like Tomasz suggests, also 
nicely fixes the problem of s3c24xx [0] only having FUNC, DAT and PUD and some 
(gpa) not even having the PUD, which was not checked in the original patch.


Heiko

[0] patch is with Kgene currently, so should make it into 3.11
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Tomasz Figa
On Thursday 16 of May 2013 13:32:48 Doug Anderson wrote:
 Tomasz,
 
 Thanks for the review!  I'll get a new patch out either today or
 tomorrow...

OK. I will be fine to go with your patches, after addressing the comments. 
In the end it's good that you posted them, as reviewing them allowed me to 
find even better ways of doing some things than I had in mine ;) .

[snip]
 
  I wonder if you couldn't do all the saving here in a single loop over
  all 
  pin control types, like:
  unsigned int offsets = bank-type-reg_offsets;
  unsigned int widths = bank-type-fld_width;
  
  for (i = 0; i  PINCFG_TYPE_NUM; ++i)
  
  if (widths[i])
  
  bank-pm_save[i] = readl(reg + offsets[i]);
  
  The only thing not handled by this loop is second CON registers in
  banks with two of them. I can't think of any better solution for this
  other than just adding a special case after the loop.
 
 Yes, that would work.  I think it wasn't possible when I first wrote
 the code against an older code base that didn't have the arrays.  I
 can give it a shot if it doesn't make restore too bad...

OK. I wonder if resume couldn't somehow benefit from this as well, but it 
probably depends on how much it can be altered without breaking the anti-
glitch functionality.

  Now as I think of it, do CON_PDN and PUD_PDN really need to be saved?
  I
  couldn't find in the documentation if they are preserved or lost in
  sleep mode. Do you have some information on this?
 
 I remember it being important.  Running a test now.  Yes, it's
 important on exynos5250.  As an example:
 
 [   62.22] samsung-pinctrl 1140.pinctrl: Restore gpa2@f0048040
 (0x=0x2212 ch 0x)
 [   62.22] samsung-pinctrl 1140.pinctrl: Restore gpb0@f0048060
 CON_PDN (0x=0x02aa)
 [   62.22] samsung-pinctrl 1140.pinctrl: Restore gpb0@f0048060
 PUD_PDN (0x=0x0155)

OK. It's good to know.

  I wonder if the whole restoration procedure couldn't be simplified. I
  don't remember my version being so complicated, but I don't have my
  patch on my screen at the moment, so I might be wrong on this.
 
 I debated about this a bunch.  Perhaps I should just delete it.  I saw
 that it was there in the old 2-bit code and it also seemed quite
 reasonable, so I kept it.  Things seem to work OK without it, but most
 things are pretty tolerant to their lines glitching (or even driving
 high to low for a short period of time).
 
 ...but your question made me check again.
 
 From previous experimentation I'm pretty certain that most pins on the
 exynos are held in the powerdown state even during early bootup of the
 SoC.  The hope is that they are released from powerdown _after_ the
 GPIO init is called.  If not then we're already glitching somewhat as
 we transition from powerdown state to default state before this
 function finally gets called.
 
 Looking at exynos, that's probably done in exynos_pm_resume(), maybe
 by mucking with the pad retention options?
 
 Oh, that probably means taht no_irq() is too late and that I need to
 figure out how to get my code called earlier...  The
 exynos_pm_resume() is called by syscore.

How all of this works is basically a good question. I couldn't find any 
mention about pins switching from power down to normal mode in the 
documentation, but maybe there is a small side note somewhere, which I 
could miss.

On S3C6410, for example, there are two modes. State is switched to power 
down mode automatically, but can be switched out either automatically on 
wake-up (exact timing is unknown to me) or by clearing a special bit, 
depending on value of other special bit.

IMHO this is rather important, so we should find out how it work on other 
SoCs and make the code account for it.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
Tomasz,

On Thu, May 16, 2013 at 2:27 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 OK. I will be fine to go with your patches, after addressing the comments.
 In the end it's good that you posted them, as reviewing them allowed me to
 find even better ways of doing some things than I had in mine ;) .

Yes.  I often find that the best way to review code is to think about
how I would implement it myself.  Certainly I think we've ended up
with something better / less buggy this way.  ;)


 How all of this works is basically a good question. I couldn't find any
 mention about pins switching from power down to normal mode in the
 documentation, but maybe there is a small side note somewhere, which I
 could miss.

 On S3C6410, for example, there are two modes. State is switched to power
 down mode automatically, but can be switched out either automatically on
 wake-up (exact timing is unknown to me) or by clearing a special bit,
 depending on value of other special bit.

 IMHO this is rather important, so we should find out how it work on other
 SoCs and make the code account for it.

Agreed that it's important.  ...but it's also good not to have tons of
complexity when it's not needed.  It sounds like S3C6410 could be
handled OK by just using the special bits and waiting to take things
out of power down mode.

...thinking about it, all SoCs that have power down modes (which you
_must_ have if your pinctrl state is lost across a low power) would be
slightly broken if they didn't have a bit to switch out of power down
mode.  Otherwise you're asking for at least some type of glitch
because you'll end up in the default state of pins for a little while
during resume.

That's not to say that there aren't broken SoCs out there and it's
entirely possible that people even designed systems around them
(knowing that the default state of each pin after wakeup is not
harmful to whatever is connected to that pin).  If there are any cases
like this then they would need the special code like my V1 patch had.
Do you know of any SoCs like this that we need to support on kernel
3.10 and higher?


I'm planning on going back to the simpler code for my next patchset
unless I can find a problem with it.


-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Tomasz Figa
On Thursday 16 of May 2013 14:51:53 Doug Anderson wrote:
 Tomasz,
 
 On Thu, May 16, 2013 at 2:27 PM, Tomasz Figa tomasz.f...@gmail.com 
wrote:
  OK. I will be fine to go with your patches, after addressing the
  comments. In the end it's good that you posted them, as reviewing
  them allowed me to find even better ways of doing some things than I
  had in mine ;) .
 Yes.  I often find that the best way to review code is to think about
 how I would implement it myself.  Certainly I think we've ended up
 with something better / less buggy this way.  ;)
 
  How all of this works is basically a good question. I couldn't find
  any
  mention about pins switching from power down to normal mode in the
  documentation, but maybe there is a small side note somewhere, which I
  could miss.
  
  On S3C6410, for example, there are two modes. State is switched to
  power down mode automatically, but can be switched out either
  automatically on wake-up (exact timing is unknown to me) or by
  clearing a special bit, depending on value of other special bit.
  
  IMHO this is rather important, so we should find out how it work on
  other SoCs and make the code account for it.
 
 Agreed that it's important.  ...but it's also good not to have tons of
 complexity when it's not needed.  It sounds like S3C6410 could be
 handled OK by just using the special bits and waiting to take things
 out of power down mode.
 
 ...thinking about it, all SoCs that have power down modes (which you
 _must_ have if your pinctrl state is lost across a low power) would be
 slightly broken if they didn't have a bit to switch out of power down
 mode.  Otherwise you're asking for at least some type of glitch
 because you'll end up in the default state of pins for a little while
 during resume.
 
 That's not to say that there aren't broken SoCs out there and it's
 entirely possible that people even designed systems around them
 (knowing that the default state of each pin after wakeup is not
 harmful to whatever is connected to that pin).  If there are any cases
 like this then they would need the special code like my V1 patch had.
 Do you know of any SoCs like this that we need to support on kernel
 3.10 and higher?

Hmm, I just checked documentation of S3C2440 and S3C2416 they seem to 
retain GPIO settings completely in sleep mode. This would mean that they 
don't require any suspend/resume support in pinctrl driver. Heiko, can you 
confirm this?

Best regards,
Tomasz

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()

2013-05-16 Thread Doug Anderson
Tomasz,

On Thu, May 16, 2013 at 12:26 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote:
 From: Prathyush K prathyus...@samsung.com

 Add the irq_set_wake function for exynos pinctrl to configure the
 external interrupt wakeup mask register.

 [dianders: minor nit fixes; port to ToT]

 Signed-off-by: Prathyush K prathyus...@samsung.com
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/pinctrl/pinctrl-exynos.c  | 45
 ---
 drivers/pinctrl/pinctrl-exynos.h  |  3 ++-
  drivers/pinctrl/pinctrl-samsung.h |  2 ++
  3 files changed, 37 insertions(+), 13 deletions(-)

 diff --git a/drivers/pinctrl/pinctrl-exynos.c
 b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644
 --- a/drivers/pinctrl/pinctrl-exynos.c
 +++ b/drivers/pinctrl/pinctrl-exynos.c
 @@ -30,6 +30,8 @@
  #include linux/spinlock.h
  #include linux/err.h

 +#include plat/pm.h
 +

 This is not going to work with CONFIG_MULTIPLATFORM.

Hmm, this sounds like it might be a bit of a long path, especially
since I haven't been keeping up with what's been going on with
MULTIPLATFORM and I'm currently midway through making 3.8 work (which
has no MULTIPLATFORM).

Perhaps for this patch it makes more sense for you to post your
version and I can review it?  We may end up just keeping our version
of this patch for 3.8 and pick up yours when we do our next rebase.
Does that sound OK?


 -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs)   \
 +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\
   {   \
   .type   = bank_type_alive, \
   .pctl_offset= reg,  \
   .nr_pins= pins, \
   .eint_type  = EINT_TYPE_WKUP,   \
   .eint_offset= offs, \
 + .eint_base  = base, \

 I can't look at my patch at the moment, but I think I have managed to get
 EINT index without adding this extra field.

It looks like this is always 2 * eint_offset in the code above.  Maybe
you just multiplied?  The multiplication works fine although I think
specifying eint_base like this might be more generic and handle future
chips better?  Ya never know...

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Heiko Stübner
Am Freitag, 17. Mai 2013, 00:08:34 schrieb Tomasz Figa:
 On Thursday 16 of May 2013 14:51:53 Doug Anderson wrote:
  Tomasz,
  
  On Thu, May 16, 2013 at 2:27 PM, Tomasz Figa tomasz.f...@gmail.com
 
 wrote:
   OK. I will be fine to go with your patches, after addressing the
   comments. In the end it's good that you posted them, as reviewing
   them allowed me to find even better ways of doing some things than I
   had in mine ;) .
  
  Yes.  I often find that the best way to review code is to think about
  how I would implement it myself.  Certainly I think we've ended up
  with something better / less buggy this way.  ;)
  
   How all of this works is basically a good question. I couldn't find
   any
   mention about pins switching from power down to normal mode in the
   documentation, but maybe there is a small side note somewhere, which I
   could miss.
   
   On S3C6410, for example, there are two modes. State is switched to
   power down mode automatically, but can be switched out either
   automatically on wake-up (exact timing is unknown to me) or by
   clearing a special bit, depending on value of other special bit.
   
   IMHO this is rather important, so we should find out how it work on
   other SoCs and make the code account for it.
  
  Agreed that it's important.  ...but it's also good not to have tons of
  complexity when it's not needed.  It sounds like S3C6410 could be
  handled OK by just using the special bits and waiting to take things
  out of power down mode.
  
  ...thinking about it, all SoCs that have power down modes (which you
  _must_ have if your pinctrl state is lost across a low power) would be
  slightly broken if they didn't have a bit to switch out of power down
  mode.  Otherwise you're asking for at least some type of glitch
  because you'll end up in the default state of pins for a little while
  during resume.
  
  That's not to say that there aren't broken SoCs out there and it's
  entirely possible that people even designed systems around them
  (knowing that the default state of each pin after wakeup is not
  harmful to whatever is connected to that pin).  If there are any cases
  like this then they would need the special code like my V1 patch had.
  Do you know of any SoCs like this that we need to support on kernel
  3.10 and higher?
 
 Hmm, I just checked documentation of S3C2440 and S3C2416 they seem to
 retain GPIO settings completely in sleep mode. This would mean that they
 don't require any suspend/resume support in pinctrl driver. Heiko, can you
 confirm this?

Hmm, my system does not have a working suspend right now, but looking at the 
legacy code (mach-s3c24xx/pm.c, etc) tells me that the gpio banks were never 
saved during suspend.

And as there were (and still are) systems with working suspend around, I'd 
assume that you're correct that the pins retain their state.

Is the same true for the s3c64xx, as I didn't find any gpio suspend handling 
for it either.


Heiko
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()

2013-05-16 Thread Tomasz Figa
On Thursday 16 of May 2013 15:25:15 Doug Anderson wrote:
 Tomasz,
 
 On Thu, May 16, 2013 at 12:26 PM, Tomasz Figa tomasz.f...@gmail.com 
wrote:
  On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote:
  From: Prathyush K prathyus...@samsung.com
  
  Add the irq_set_wake function for exynos pinctrl to configure the
  external interrupt wakeup mask register.
  
  [dianders: minor nit fixes; port to ToT]
  
  Signed-off-by: Prathyush K prathyus...@samsung.com
  Signed-off-by: Doug Anderson diand...@chromium.org
  ---
  
   drivers/pinctrl/pinctrl-exynos.c  | 45
  
  ---
  drivers/pinctrl/pinctrl-exynos.h  |  3 ++-
  
   drivers/pinctrl/pinctrl-samsung.h |  2 ++
   3 files changed, 37 insertions(+), 13 deletions(-)
  
  diff --git a/drivers/pinctrl/pinctrl-exynos.c
  b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644
  --- a/drivers/pinctrl/pinctrl-exynos.c
  +++ b/drivers/pinctrl/pinctrl-exynos.c
  @@ -30,6 +30,8 @@
  
   #include linux/spinlock.h
   #include linux/err.h
  
  +#include plat/pm.h
  +
  
  This is not going to work with CONFIG_MULTIPLATFORM.
 
 Hmm, this sounds like it might be a bit of a long path, especially
 since I haven't been keeping up with what's been going on with
 MULTIPLATFORM and I'm currently midway through making 3.8 work (which
 has no MULTIPLATFORM).

Well, to make long story short, including headers from plat/ and mach/ 
from files outside plat/ or mach/ is no longer valid with 
CONFIG_MULTIPLATFORM, because more than one plat and/or mach can be 
enabled at the same time. In addition to this, care must be taken for code 
to not break platforms other than written for, when compiled into the 
resulting kernel.

 Perhaps for this patch it makes more sense for you to post your
 version and I can review it?  We may end up just keeping our version
 of this patch for 3.8 and pick up yours when we do our next rebase.
 Does that sound OK?

Fine. I will also send a patch adding save and restore for several EINT 
registers that need it.

  -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs)   \
  +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\
  
{   \

.type   = bank_type_alive, \
.pctl_offset= reg,  \
.nr_pins= pins, \
.eint_type  = EINT_TYPE_WKUP,   \
.eint_offset= offs, \
  
  + .eint_base  = base, \
  
  I can't look at my patch at the moment, but I think I have managed to
  get EINT index without adding this extra field.
 
 It looks like this is always 2 * eint_offset in the code above.  Maybe
 you just multiplied?  The multiplication works fine although I think
 specifying eint_base like this might be more generic and handle future
 chips better?  Ya never know...

Since EINT handling is highly SoC-specific (i.e. done in pinctrl-exynos, 
not pinctrl-samsung), such assumption wouldn't be a problem. Let me see 
how I solved this problem in my version tomorrow at work.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Tomasz Figa
On Friday 17 of May 2013 00:30:38 Heiko Stübner wrote:
 Am Freitag, 17. Mai 2013, 00:08:34 schrieb Tomasz Figa:
  On Thursday 16 of May 2013 14:51:53 Doug Anderson wrote:
   Tomasz,
   
   On Thu, May 16, 2013 at 2:27 PM, Tomasz Figa tomasz.f...@gmail.com
  
  wrote:
OK. I will be fine to go with your patches, after addressing the
comments. In the end it's good that you posted them, as reviewing
them allowed me to find even better ways of doing some things than
I
had in mine ;) .
   
   Yes.  I often find that the best way to review code is to think
   about
   how I would implement it myself.  Certainly I think we've ended up
   with something better / less buggy this way.  ;)
   
How all of this works is basically a good question. I couldn't
find
any
mention about pins switching from power down to normal mode in the
documentation, but maybe there is a small side note somewhere,
which I
could miss.

On S3C6410, for example, there are two modes. State is switched to
power down mode automatically, but can be switched out either
automatically on wake-up (exact timing is unknown to me) or by
clearing a special bit, depending on value of other special bit.

IMHO this is rather important, so we should find out how it work
on
other SoCs and make the code account for it.
   
   Agreed that it's important.  ...but it's also good not to have tons
   of
   complexity when it's not needed.  It sounds like S3C6410 could be
   handled OK by just using the special bits and waiting to take things
   out of power down mode.
   
   ...thinking about it, all SoCs that have power down modes (which you
   _must_ have if your pinctrl state is lost across a low power) would
   be
   slightly broken if they didn't have a bit to switch out of power
   down
   mode.  Otherwise you're asking for at least some type of glitch
   because you'll end up in the default state of pins for a little
   while
   during resume.
   
   That's not to say that there aren't broken SoCs out there and it's
   entirely possible that people even designed systems around them
   (knowing that the default state of each pin after wakeup is not
   harmful to whatever is connected to that pin).  If there are any
   cases
   like this then they would need the special code like my V1 patch
   had.
   Do you know of any SoCs like this that we need to support on kernel
   3.10 and higher?
  
  Hmm, I just checked documentation of S3C2440 and S3C2416 they seem to
  retain GPIO settings completely in sleep mode. This would mean that
  they don't require any suspend/resume support in pinctrl driver.
  Heiko, can you confirm this?
 
 Hmm, my system does not have a working suspend right now, but looking at
 the legacy code (mach-s3c24xx/pm.c, etc) tells me that the gpio banks
 were never saved during suspend.
 
 And as there were (and still are) systems with working suspend around,
 I'd assume that you're correct that the pins retain their state.
 
 Is the same true for the s3c64xx, as I didn't find any gpio suspend
 handling for it either.

Seems like I need some sleep, as I'm already starting to overlook large 
blobs of code. 

Originally, GPIO suspend/resume handlers have been configured in 
drivers/gpio/gpio-samsung.c, by setting pm field of samsung_gpio_chip 
struct to point to appropriate samsung_gpio_pm struct, which contains 
pointers to save and resume callbacks.

In result, samsung_gpio_pm_2bit_* or samsung_gpio_pm_4bit_* have been 
used, depending on bank type, on all SoCs.

Now since the documentation states that wake-up reset doesn't reset GPIO 
registers (at least on S3C2440 and S3C2416), I wonder what is the correct 
way of handling them.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
Tomasz,

On Thu, May 16, 2013 at 3:56 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 Seems like I need some sleep, as I'm already starting to overlook large
 blobs of code.

 Originally, GPIO suspend/resume handlers have been configured in
 drivers/gpio/gpio-samsung.c, by setting pm field of samsung_gpio_chip
 struct to point to appropriate samsung_gpio_pm struct, which contains
 pointers to save and resume callbacks.

 In result, samsung_gpio_pm_2bit_* or samsung_gpio_pm_4bit_* have been
 used, depending on bank type, on all SoCs.

 Now since the documentation states that wake-up reset doesn't reset GPIO
 registers (at least on S3C2440 and S3C2416), I wonder what is the correct
 way of handling them.

If state of these registers isn't lost on those SoCs then running the
save/restore shouldn't _hurt_ though, right?  If you can run the old
GPIO code on one of those systems and do a suspend/resume you could
check...

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
The GPIO states need to be restored after s2r and this is not currently
supported in the pinctrl driver. This patch saves the gpio states before
suspend and restores them after resume.

Saving and restoring is done very early using syscore_ops and must
happen before pins are released from their powerdown state.

Patch originally from Prathyush K prathyus...@samsung.com but
rewritten by Doug Anderson diand...@chromium.org.

Signed-off-by: Prathyush K prathyus...@samsung.com
Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Now uses sycore_ops to make sure we're early enough.
- Try to handle two CON registers better.
- Should handle s3c24xx better as per Heiko.
- Simpler code; no longer tries to avoid glitching lines since
  we _think_ all current SoCs should have pins in power down state
  when the restore is called.
- Dropped eint patch for now; Tomasz will post his version.

 drivers/pinctrl/pinctrl-samsung.c | 140 ++
 drivers/pinctrl/pinctrl-samsung.h |   5 ++
 2 files changed, 145 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 9763668..851eada 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -28,6 +28,7 @@
 #include linux/gpio.h
 #include linux/irqdomain.h
 #include linux/spinlock.h
+#include linux/syscore_ops.h
 
 #include core.h
 #include pinctrl-samsung.h
@@ -48,6 +49,9 @@ static struct pin_config {
{ samsung,pin-pud-pdn, PINCFG_TYPE_PUD_PDN },
 };
 
+/* Global list of devices (struct samsung_pinctrl_drv_data) */
+LIST_HEAD(drvdata_list);
+
 static unsigned int pin_base;
 
 static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
@@ -961,9 +965,137 @@ static int samsung_pinctrl_probe(struct platform_device 
*pdev)
ctrl-eint_wkup_init(drvdata);
 
platform_set_drvdata(pdev, drvdata);
+
+   /* Add to the global list */
+   list_add_tail(drvdata-node, drvdata_list);
+
return 0;
 }
 
+#ifdef CONFIG_PM
+
+/**
+ * samsung_pinctrl_suspend_dev - save pinctrl state for suspend for a device
+ *
+ * Save data for all banks handled by this device.
+ */
+static void samsung_pinctrl_suspend_dev(
+   struct samsung_pinctrl_drv_data *drvdata)
+{
+   struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
+   void __iomem *virt_base = drvdata-virt_base;
+   int i;
+
+   for (i = 0; i  ctrl-nr_banks; i++) {
+   struct samsung_pin_bank *bank = ctrl-pin_banks[i];
+   void __iomem *reg = virt_base + bank-pctl_offset;
+
+   u8 *offs = bank-type-reg_offset;
+   u8 *widths = bank-type-fld_width;
+   enum pincfg_type type;
+
+   for (type = 0; type  PINCFG_TYPE_NUM; type++)
+   if (widths[type])
+   bank-pm_save[type] = readl(reg + offs[type]);
+
+   if (widths[PINCFG_TYPE_FUNC] * bank-nr_pins  32) {
+   /* Some banks have two config registers */
+   bank-pm_save[PINCFG_TYPE_NUM] =
+   readl(reg + offs[PINCFG_TYPE_FUNC] + 4);
+   pr_debug(Save %s @ %p (con %#010x %08x)\n,
+bank-name, reg,
+bank-pm_save[PINCFG_TYPE_FUNC],
+bank-pm_save[PINCFG_TYPE_NUM]);
+   } else {
+   pr_debug(Save %s @ %p (con %#010x)\n, bank-name,
+reg, bank-pm_save[PINCFG_TYPE_FUNC]);
+   }
+   }
+}
+
+/**
+ * samsung_pinctrl_resume_dev - restore pinctrl state from suspend for a device
+ *
+ * Restore one of the banks that was saved during suspend.
+ *
+ * We don't bother doing anything complicated to avoid glitching lines since
+ * we're called before pad retention is turned off.
+ */
+static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data 
*drvdata)
+{
+   struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
+   void __iomem *virt_base = drvdata-virt_base;
+   int i;
+
+   for (i = 0; i  ctrl-nr_banks; i++) {
+   struct samsung_pin_bank *bank = ctrl-pin_banks[i];
+   void __iomem *reg = virt_base + bank-pctl_offset;
+
+   u8 *offs = bank-type-reg_offset;
+   u8 *widths = bank-type-fld_width;
+   enum pincfg_type type;
+
+   if (widths[PINCFG_TYPE_FUNC] * bank-nr_pins  32) {
+   /* Some banks have two config registers */
+   pr_debug(%s @ %p (con %#010x %08x = %#010x %08x)\n,
+bank-name, reg,
+readl(reg + offs[PINCFG_TYPE_FUNC]),
+readl(reg + offs[PINCFG_TYPE_FUNC] + 4),
+bank-pm_save[PINCFG_TYPE_FUNC],
+bank-pm_save[PINCFG_TYPE_NUM]);
+   

Re: [PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()

2013-05-16 Thread Doug Anderson
Tomasz,

On Thu, May 16, 2013 at 3:37 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 Well, to make long story short, including headers from plat/ and mach/
 from files outside plat/ or mach/ is no longer valid with
 CONFIG_MULTIPLATFORM, because more than one plat and/or mach can be
 enabled at the same time. In addition to this, care must be taken for code
 to not break platforms other than written for, when compiled into the
 resulting kernel.

Right.  That makes sense.  That's also why it would be a bit of a
longshot for me to get this done right now.  I'd imagine that there
would be a number of changes to the samsung pm infrastructure that are
needed to make this work and I don't have all of those in my tree
right now.  We've already picked back a lot to 3.8, but multiplatform
seems too much.


 Perhaps for this patch it makes more sense for you to post your
 version and I can review it?  We may end up just keeping our version
 of this patch for 3.8 and pick up yours when we do our next rebase.
 Does that sound OK?

 Fine. I will also send a patch adding save and restore for several EINT
 registers that need it.

OK, sounds good.  I was trying to figure out why we didn't seem to
have those in our 3.4 stuff and that it seems to work without
saving/restoring.  I assumed that maybe higher level code was
masking/unmasking interrupts but didn't dig.


 Since EINT handling is highly SoC-specific (i.e. done in pinctrl-exynos,
 not pinctrl-samsung), such assumption wouldn't be a problem. Let me see
 how I solved this problem in my version tomorrow at work.

Fair enough.  :)  Looking forward to seeing your patch!

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
Tomasz,

On Thu, May 16, 2013 at 4:10 PM, Doug Anderson diand...@chromium.org wrote:
 If state of these registers isn't lost on those SoCs then running the
 save/restore shouldn't _hurt_ though, right?  If you can run the old
 GPIO code on one of those systems and do a suspend/resume you could
 check...

I think it's been too long of a day for me, too.

I just thought about this and realized that there is no powerdown
registers for the GPX banks on exynos5250.  ...and they don't lose
their state at sleep!  ...so maybe a reasonable thing to do would be
to skip save/restore in any case where there are no powerdown
registers?

You can see a printout in my case:

[  412.84] gpx0 @ f004ac00 (con 0x3110 = 0x3110)
[  412.84] gpx1 @ f004ac20 (con 0x1f10ff10 = 0x1f10ff10)
[  412.84] gpx2 @ f004ac40 (con 0x1f000f0f = 0x1f000f0f)
[  412.84] gpx3 @ f004ac60 (con 0x00f00f01 = 0x00f00f01)

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] pinctrl: samsung: fix suspend/resume functionality

2013-05-16 Thread Doug Anderson
The GPIO states need to be restored after s2r and this is not currently
supported in the pinctrl driver. This patch saves the gpio states before
suspend and restores them after resume.

Saving and restoring is done very early using syscore_ops and must
happen before pins are released from their powerdown state.

Patch originally from Prathyush K prathyus...@samsung.com but
rewritten by Doug Anderson diand...@chromium.org.

Signed-off-by: Prathyush K prathyus...@samsung.com
Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v3:
- Skip save and restore for banks with no powerdown config.

Changes in v2:
- Now uses sycore_ops to make sure we're early enough.
- Try to handle two CON registers better.
- Should handle s3c24xx better as per Heiko.
- Simpler code; no longer tries to avoid glitching lines since
  we _think_ all current SoCs should have pins in power down state
  when the restore is called.
- Dropped eint patch for now; Tomasz will post his version.

 drivers/pinctrl/pinctrl-samsung.c | 148 ++
 drivers/pinctrl/pinctrl-samsung.h |   5 ++
 2 files changed, 153 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 9763668..d45e36f 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -28,6 +28,7 @@
 #include linux/gpio.h
 #include linux/irqdomain.h
 #include linux/spinlock.h
+#include linux/syscore_ops.h
 
 #include core.h
 #include pinctrl-samsung.h
@@ -48,6 +49,9 @@ static struct pin_config {
{ samsung,pin-pud-pdn, PINCFG_TYPE_PUD_PDN },
 };
 
+/* Global list of devices (struct samsung_pinctrl_drv_data) */
+LIST_HEAD(drvdata_list);
+
 static unsigned int pin_base;
 
 static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
@@ -961,9 +965,145 @@ static int samsung_pinctrl_probe(struct platform_device 
*pdev)
ctrl-eint_wkup_init(drvdata);
 
platform_set_drvdata(pdev, drvdata);
+
+   /* Add to the global list */
+   list_add_tail(drvdata-node, drvdata_list);
+
return 0;
 }
 
+#ifdef CONFIG_PM
+
+/**
+ * samsung_pinctrl_suspend_dev - save pinctrl state for suspend for a device
+ *
+ * Save data for all banks handled by this device.
+ */
+static void samsung_pinctrl_suspend_dev(
+   struct samsung_pinctrl_drv_data *drvdata)
+{
+   struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
+   void __iomem *virt_base = drvdata-virt_base;
+   int i;
+
+   for (i = 0; i  ctrl-nr_banks; i++) {
+   struct samsung_pin_bank *bank = ctrl-pin_banks[i];
+   void __iomem *reg = virt_base + bank-pctl_offset;
+
+   u8 *offs = bank-type-reg_offset;
+   u8 *widths = bank-type-fld_width;
+   enum pincfg_type type;
+
+   /* Registers without a powerdown config aren't lost */
+   if (!widths[PINCFG_TYPE_CON_PDN])
+   continue;
+
+   for (type = 0; type  PINCFG_TYPE_NUM; type++)
+   if (widths[type])
+   bank-pm_save[type] = readl(reg + offs[type]);
+
+   if (widths[PINCFG_TYPE_FUNC] * bank-nr_pins  32) {
+   /* Some banks have two config registers */
+   bank-pm_save[PINCFG_TYPE_NUM] =
+   readl(reg + offs[PINCFG_TYPE_FUNC] + 4);
+   pr_debug(Save %s @ %p (con %#010x %08x)\n,
+bank-name, reg,
+bank-pm_save[PINCFG_TYPE_FUNC],
+bank-pm_save[PINCFG_TYPE_NUM]);
+   } else {
+   pr_debug(Save %s @ %p (con %#010x)\n, bank-name,
+reg, bank-pm_save[PINCFG_TYPE_FUNC]);
+   }
+   }
+}
+
+/**
+ * samsung_pinctrl_resume_dev - restore pinctrl state from suspend for a device
+ *
+ * Restore one of the banks that was saved during suspend.
+ *
+ * We don't bother doing anything complicated to avoid glitching lines since
+ * we're called before pad retention is turned off.
+ */
+static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data 
*drvdata)
+{
+   struct samsung_pin_ctrl *ctrl = drvdata-ctrl;
+   void __iomem *virt_base = drvdata-virt_base;
+   int i;
+
+   for (i = 0; i  ctrl-nr_banks; i++) {
+   struct samsung_pin_bank *bank = ctrl-pin_banks[i];
+   void __iomem *reg = virt_base + bank-pctl_offset;
+
+   u8 *offs = bank-type-reg_offset;
+   u8 *widths = bank-type-fld_width;
+   enum pincfg_type type;
+
+   /* Registers without a powerdown config aren't lost */
+   if (!widths[PINCFG_TYPE_CON_PDN])
+   continue;
+
+   if (widths[PINCFG_TYPE_FUNC] * bank-nr_pins  32) {
+   /* Some banks have two config registers */
+