diff mbox series

[01/36] ARM: samsung: make S3C24XX_MISCCR access indirect

Message ID 20191010203043.1241612-1-arnd@arndb.de
State Accepted
Commit 346f183cc1348cbf3814796afab0ea34475152a9
Headers show
Series [01/36] ARM: samsung: make S3C24XX_MISCCR access indirect | expand

Commit Message

Arnd Bergmann Oct. 10, 2019, 8:29 p.m. UTC 
The clk driver uses both a function call into an exported
platform file and a direct register access to a hardcoded
virtual address for accessing the MISCCR register, both
become are a problem for a multiplatform kernel because
of the header file dependency.

Make this an indirect function call through platform data
instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 arch/arm/mach-s3c24xx/common.c         |  3 +++
 drivers/clk/samsung/clk-s3c2410-dclk.c | 10 ++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
2.20.0

Comments

Krzysztof Kozlowski Oct. 23, 2019, 10:27 a.m. UTC | #1 
On Thu, Oct 10, 2019 at 10:29:49PM +0200, Arnd Bergmann wrote:
> The pm-debug code is one of the few things shared between s3c24xx/s3c64xx

> and the newer s5pv210. In order to make s5pv210 independent of plat-samsung,

> change the common bits of this code to no longer reference the s3c specific

> bits.

> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  arch/arm/mach-s3c24xx/include/mach/pm-core.h  |  7 +--

>  arch/arm/mach-s3c64xx/include/mach/pm-core.h  | 44 +---------------

>  arch/arm/mach-s3c64xx/pm.c                    | 50 +++++++++++++++++++

>  arch/arm/mach-s5pv210/pm.c                    |  6 +--

>  .../arm/plat-samsung/include/plat/pm-common.h | 29 +++++------

>  arch/arm/plat-samsung/pm-debug.c              | 23 ++-------

>  arch/arm/plat-samsung/pm.c                    | 10 ++--

>  7 files changed, 80 insertions(+), 89 deletions(-)

> 

> diff --git a/arch/arm/mach-s3c24xx/include/mach/pm-core.h b/arch/arm/mach-s3c24xx/include/mach/pm-core.h

> index 5e4ce89d0158..8f87606c4cdc 100644

> --- a/arch/arm/mach-s3c24xx/include/mach/pm-core.h

> +++ b/arch/arm/mach-s3c24xx/include/mach/pm-core.h

> @@ -15,6 +15,7 @@

>  

>  static inline void s3c_pm_debug_init_uart(void)

>  {

> +#ifdef CONFIG_SAMSUNG_PM_DEBUG

>  	unsigned long tmp = __raw_readl(S3C2410_CLKCON);

>  

>  	/* re-start uart clocks */

> @@ -24,6 +25,7 @@ static inline void s3c_pm_debug_init_uart(void)

>  

>  	__raw_writel(tmp, S3C2410_CLKCON);

>  	udelay(10);

> +#endif

>  }

>  

>  static inline void s3c_pm_arch_prepare_irqs(void)

> @@ -75,11 +77,6 @@ static inline void s3c_pm_arch_show_resume_irqs(void)

>  				s3c_irqwake_eintmask);

>  }

>  

> -static inline void s3c_pm_arch_update_uart(void __iomem *regs,

> -					   struct pm_uart_save *save)

> -{

> -}

> -

>  static inline void s3c_pm_restored_gpios(void) { }

>  static inline void samsung_pm_saved_gpios(void) { }

>  

> diff --git a/arch/arm/mach-s3c64xx/include/mach/pm-core.h b/arch/arm/mach-s3c64xx/include/mach/pm-core.h

> index bbf79ed28583..33cf242734a0 100644

> --- a/arch/arm/mach-s3c64xx/include/mach/pm-core.h

> +++ b/arch/arm/mach-s3c64xx/include/mach/pm-core.h

> @@ -20,6 +20,7 @@

>  

>  static inline void s3c_pm_debug_init_uart(void)

>  {

> +#ifdef CONFIG_SAMSUNG_PM_DEBUG

>  	u32 tmp = __raw_readl(S3C_PCLK_GATE);

>  

>  	/* As a note, since the S3C64XX UARTs generally have multiple

> @@ -35,6 +36,7 @@ static inline void s3c_pm_debug_init_uart(void)

>  

>  	__raw_writel(tmp, S3C_PCLK_GATE);

>  	udelay(10);

> +#endif

>  }

>  

>  static inline void s3c_pm_arch_prepare_irqs(void)

> @@ -63,48 +65,6 @@ static inline void s3c_pm_arch_show_resume_irqs(void)

>  #define s3c_irqwake_intallow  0

>  #endif

>  

> -static inline void s3c_pm_arch_update_uart(void __iomem *regs,

> -					   struct pm_uart_save *save)

> -{

> -	u32 ucon = __raw_readl(regs + S3C2410_UCON);

> -	u32 ucon_clk = ucon & S3C6400_UCON_CLKMASK;

> -	u32 save_clk = save->ucon & S3C6400_UCON_CLKMASK;

> -	u32 new_ucon;

> -	u32 delta;

> -

> -	/* S3C64XX UART blocks only support level interrupts, so ensure that

> -	 * when we restore unused UART blocks we force the level interrupt

> -	 * settigs. */

> -	save->ucon |= S3C2410_UCON_TXILEVEL | S3C2410_UCON_RXILEVEL;

> -

> -	/* We have a constraint on changing the clock type of the UART

> -	 * between UCLKx and PCLK, so ensure that when we restore UCON

> -	 * that the CLK field is correctly modified if the bootloader

> -	 * has changed anything.

> -	 */

> -	if (ucon_clk != save_clk) {

> -		new_ucon = save->ucon;

> -		delta = ucon_clk ^ save_clk;

> -

> -		/* change from UCLKx => wrong PCLK,

> -		 * either UCLK can be tested for by a bit-test

> -		 * with UCLK0 */

> -		if (ucon_clk & S3C6400_UCON_UCLK0 &&

> -		    !(save_clk & S3C6400_UCON_UCLK0) &&

> -		    delta & S3C6400_UCON_PCLK2) {

> -			new_ucon &= ~S3C6400_UCON_UCLK0;

> -		} else if (delta == S3C6400_UCON_PCLK2) {

> -			/* as an precaution, don't change from

> -			 * PCLK2 => PCLK or vice-versa */

> -			new_ucon ^= S3C6400_UCON_PCLK2;

> -		}

> -

> -		S3C_PMDBG("ucon change %04x => %04x (save=%04x)\n",

> -			  ucon, new_ucon, save->ucon);

> -		save->ucon = new_ucon;

> -	}

> -}

> -

>  static inline void s3c_pm_restored_gpios(void)

>  {

>  	/* ensure sleep mode has been cleared from the system */

> diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c

> index fd6dbb263ed5..a612e9779057 100644

> --- a/arch/arm/mach-s3c64xx/pm.c

> +++ b/arch/arm/mach-s3c64xx/pm.c

> @@ -305,6 +305,56 @@ static void s3c64xx_pm_prepare(void)

>  	__raw_writel(__raw_readl(S3C64XX_WAKEUP_STAT), S3C64XX_WAKEUP_STAT);

>  }

>  

> +#ifdef CONFIG_SAMSUNG_PM_DEBUG

> +void s3c_pm_arch_update_uart(void __iomem *regs, struct pm_uart_save *save)

> +{

> +	u32 ucon;

> +	u32 ucon_clk

> +	u32 save_clk;

> +	u32 new_ucon;

> +	u32 delta;

> +

> +	if (!soc_is_s3c64xx())

> +		return;

> +

> +	ucon = __raw_readl(regs + S3C2410_UCON);

> +	ucon_clk = ucon & S3C6400_UCON_CLKMASK;

> +	sav_clk = save->ucon & S3C6400_UCON_CLKMASK;

> +

> +	/* S3C64XX UART blocks only support level interrupts, so ensure that

> +	 * when we restore unused UART blocks we force the level interrupt

> +	 * settigs. */

> +	save->ucon |= S3C2410_UCON_TXILEVEL | S3C2410_UCON_RXILEVEL;

> +

> +	/* We have a constraint on changing the clock type of the UART

> +	 * between UCLKx and PCLK, so ensure that when we restore UCON

> +	 * that the CLK field is correctly modified if the bootloader

> +	 * has changed anything.

> +	 */

> +	if (ucon_clk != save_clk) {

> +		new_ucon = save->ucon;

> +		delta = ucon_clk ^ save_clk;

> +

> +		/* change from UCLKx => wrong PCLK,

> +		 * either UCLK can be tested for by a bit-test

> +		 * with UCLK0 */

> +		if (ucon_clk & S3C6400_UCON_UCLK0 &&

> +		    !(save_clk & S3C6400_UCON_UCLK0) &&

> +		    delta & S3C6400_UCON_PCLK2) {

> +			new_ucon &= ~S3C6400_UCON_UCLK0;

> +		} else if (delta == S3C6400_UCON_PCLK2) {

> +			/* as an precaution, don't change from

> +			 * PCLK2 => PCLK or vice-versa */

> +			new_ucon ^= S3C6400_UCON_PCLK2;

> +		}

> +

> +		S3C_PMDBG("ucon change %04x => %04x (save=%04x)\n",

> +			  ucon, new_ucon, save->ucon);

> +		save->ucon = new_ucon;

> +	}

> +}

> +#endif

> +

>  int __init s3c64xx_pm_init(void)

>  {

>  	int i;

> diff --git a/arch/arm/mach-s5pv210/pm.c b/arch/arm/mach-s5pv210/pm.c

> index b336df0c57f3..efdb5a27c060 100644

> --- a/arch/arm/mach-s5pv210/pm.c

> +++ b/arch/arm/mach-s5pv210/pm.c

> @@ -99,8 +99,6 @@ static int s5pv210_suspend_enter(suspend_state_t state)

>  	u32 eint_wakeup_mask = s5pv210_read_eint_wakeup_mask();

>  	int ret;

>  

> -	s3c_pm_debug_init();


Your patch is not equivalent here. If there is a reason behind removal
of UART init (e.g. not needed), I prefer to make it in separate patch.

Rest looks good.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 23, 2019, 12:46 p.m. UTC | #2 
On Thu, Oct 10, 2019 at 10:29:47PM +0200, Arnd Bergmann wrote:
> The resources are correctly initialized, so just use them

> instead of relying on hardcoded data from platform headers.


Generic comment to all patches - you seem to break commit msg lines
slightly too early. In certain cases it makes them unnecessarily longer.
Maybe your editor has to be fixed to wrap at 75 column.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 23, 2019, 12:50 p.m. UTC | #3 
On Thu, Oct 10, 2019 at 10:30:05PM +0200, Arnd Bergmann wrote:
> The s3c_gpio_cfgall_range() function is an internal interface of

> the samsung gpio driver and should not be called directly by drivers,

> so move the iis pin initialization into the boards.

> 

> Note that the s3c2412-i2s driver has no boards using it in

> mainline linux, the driver gets selected for the jive machine

> but is never instantiated.

> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  arch/arm/mach-s3c24xx/mach-gta02.c    | 4 ++++

>  arch/arm/mach-s3c24xx/mach-h1940.c    | 3 +++

>  arch/arm/mach-s3c24xx/mach-mini2440.c | 5 +++++

>  arch/arm/mach-s3c24xx/mach-n30.c      | 5 +++++

>  arch/arm/mach-s3c24xx/mach-nexcoder.c | 5 +++++

>  arch/arm/mach-s3c24xx/mach-otom.c     | 6 ++++++

>  arch/arm/mach-s3c24xx/mach-qt2410.c   | 3 +++

>  arch/arm/mach-s3c24xx/mach-rx1950.c   | 3 +++

>  arch/arm/mach-s3c24xx/mach-rx3715.c   | 4 ++++

>  arch/arm/mach-s3c24xx/mach-smdk2410.c | 5 +++++

>  arch/arm/mach-s3c24xx/mach-smdk2413.c | 4 ++++

>  arch/arm/mach-s3c24xx/mach-smdk2440.c | 6 +++++-

>  arch/arm/mach-s3c24xx/mach-vstms.c    | 6 +++++-

>  arch/arm/mach-s3c24xx/simtec-audio.c  | 6 ++++++

>  sound/soc/samsung/s3c2412-i2s.c       | 7 -------

>  sound/soc/samsung/s3c24xx-i2s.c       | 7 -------

>  16 files changed, 63 insertions(+), 16 deletions(-)

> 

> diff --git a/arch/arm/mach-s3c24xx/mach-gta02.c b/arch/arm/mach-s3c24xx/mach-gta02.c

> index 526fd0933289..1ca0460d82f4 100644

> --- a/arch/arm/mach-s3c24xx/mach-gta02.c

> +++ b/arch/arm/mach-s3c24xx/mach-gta02.c

> @@ -540,6 +540,10 @@ static void __init gta02_machine_init(void)

>  

>  	i2c_register_board_info(0, gta02_i2c_devs, ARRAY_SIZE(gta02_i2c_devs));

>  

> +	/* Configure the I2S pins (GPE0...GPE4) in correct mode */

> +	s3c_gpio_cfgall_range(S3C2410_GPE(0), 5, S3C_GPIO_SFN(2),

> +			      S3C_GPIO_PULL_NONE);


This is not entirely equivalent move as before this was probe (so being
executed also on rebinds) and now it is init. I guess it should not make
any difference so let it be.

Best regards,
Krzysztof
Arnd Bergmann Oct. 23, 2019, 1:26 p.m. UTC | #4 
On Wed, Oct 23, 2019 at 2:47 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>

> On Thu, Oct 10, 2019 at 10:29:47PM +0200, Arnd Bergmann wrote:

> > The resources are correctly initialized, so just use them

> > instead of relying on hardcoded data from platform headers.

>

> Generic comment to all patches - you seem to break commit msg lines

> slightly too early. In certain cases it makes them unnecessarily longer.

> Maybe your editor has to be fixed to wrap at 75 column.


I tend to use '/usr//bin/fmt' with the default setting. I think the idea of that
default is that an email reply with added '>' characters doesn't immediately
have to reflow the text.

       Arnd
Krzysztof Kozlowski Oct. 23, 2019, 1:44 p.m. UTC | #5 
On Thu, Oct 10, 2019 at 10:30:15PM +0200, Arnd Bergmann wrote:
> There are two identical copies of the s3c2412_cpufreq_setrefresh

> function: a static one in the cpufreq driver and a global

> version in iotiming-s3c2412.c.

> 

> As the function requires the use of a hardcoded register address

> from a header that we want to not be visible to drivers, just

> move the existing global function and add a declaration in

> one of the cpufreq header files.

> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/cpufreq/s3c2412-cpufreq.c            | 23 --------------------

>  include/linux/soc/samsung/s3c-cpufreq-core.h |  1 +

>  2 files changed, 1 insertion(+), 23 deletions(-)

> 

> diff --git a/drivers/cpufreq/s3c2412-cpufreq.c b/drivers/cpufreq/s3c2412-cpufreq.c

> index 38dc9e6db633..a77c63e92e1a 100644

> --- a/drivers/cpufreq/s3c2412-cpufreq.c

> +++ b/drivers/cpufreq/s3c2412-cpufreq.c

> @@ -25,8 +25,6 @@

>  #include <asm/mach/arch.h>

>  #include <asm/mach/map.h>

>  

> -#include <mach/s3c2412.h>

> -

>  #include <mach/map.h>

>  

>  #define S3C2410_CLKREG(x) ((x) + S3C24XX_VA_CLKPWR)

> @@ -156,27 +154,6 @@ static void s3c2412_cpufreq_setdivs(struct s3c_cpufreq_config *cfg)

>  	clk_set_parent(armclk, cfg->divs.dvs ? hclk : fclk);

>  }

>  

> -static void s3c2412_cpufreq_setrefresh(struct s3c_cpufreq_config *cfg)

> -{

> -	struct s3c_cpufreq_board *board = cfg->board;

> -	unsigned long refresh;

> -

> -	s3c_freq_dbg("%s: refresh %u ns, hclk %lu\n", __func__,

> -		     board->refresh, cfg->freq.hclk);

> -

> -	/* Reduce both the refresh time (in ns) and the frequency (in MHz)

> -	 * by 10 each to ensure that we do not overflow 32 bit numbers. This

> -	 * should work for HCLK up to 133MHz and refresh period up to 30usec.

> -	 */

> -

> -	refresh = (board->refresh / 10);

> -	refresh *= (cfg->freq.hclk / 100);

> -	refresh /= (1 * 1000 * 1000);	/* 10^6 */

> -

> -	s3c_freq_dbg("%s: setting refresh 0x%08lx\n", __func__, refresh);

> -	__raw_writel(refresh, S3C2412_REFRESH);

> -}

> -

>  /* set the default cpu frequency information, based on an 200MHz part

>   * as we have no other way of detecting the speed rating in software.

>   */

> diff --git a/include/linux/soc/samsung/s3c-cpufreq-core.h b/include/linux/soc/samsung/s3c-cpufreq-core.h

> index 4d22be1031b9..eca942559014 100644

> --- a/include/linux/soc/samsung/s3c-cpufreq-core.h

> +++ b/include/linux/soc/samsung/s3c-cpufreq-core.h

> @@ -246,6 +246,7 @@ extern int s3c2412_iotiming_calc(struct s3c_cpufreq_config *cfg,

>  

>  extern void s3c2412_iotiming_set(struct s3c_cpufreq_config *cfg,

>  				 struct s3c_iotimings *iot);

> +extern void s3c2412_cpufreq_setrefresh(struct s3c_cpufreq_config *cfg);


I think that it does not cover the !CONFIG_S3C2412_IOTIMING case.
Either you need to provide also the empty stub or add default=y to
S3C2412_IOTIMING. Otherwise cpufreq driver might end up without this.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm/mach-s3c24xx/common.c b/arch/arm/mach-s3c24xx/common.c
index 3dc029c2d2cb..ebf6bde67816 100644
--- a/arch/arm/mach-s3c24xx/common.c
+++ b/arch/arm/mach-s3c24xx/common.c
@@ -667,5 +667,8 @@  struct platform_device s3c2410_device_dclk = {
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(s3c2410_dclk_resource),
 	.resource	= s3c2410_dclk_resource,
+	.dev		= {
+		.platform_data = s3c2410_modify_misccr,
+	},
 };
 #endif
diff --git a/drivers/clk/samsung/clk-s3c2410-dclk.c b/drivers/clk/samsung/clk-s3c2410-dclk.c
index 1281672cb00e..fbcec0252c45 100644
--- a/drivers/clk/samsung/clk-s3c2410-dclk.c
+++ b/drivers/clk/samsung/clk-s3c2410-dclk.c
@@ -14,10 +14,6 @@ 
 #include <linux/module.h>
 #include "clk.h"
 
-/* legacy access to misccr, until dt conversion is finished */
-#include <mach/hardware.h>
-#include <mach/regs-gpio.h>
-
 #define MUX_DCLK0	0
 #define MUX_DCLK1	1
 #define DIV_DCLK0	2
@@ -52,6 +48,7 @@  struct s3c24xx_clkout {
 	struct clk_hw		hw;
 	u32			mask;
 	u8			shift;
+	unsigned int (*modify_misccr)(unsigned int clr, unsigned int chg);
 };
 
 #define to_s3c24xx_clkout(_hw) container_of(_hw, struct s3c24xx_clkout, hw)
@@ -62,7 +59,7 @@  static u8 s3c24xx_clkout_get_parent(struct clk_hw *hw)
 	int num_parents = clk_hw_get_num_parents(hw);
 	u32 val;
 
-	val = readl_relaxed(S3C24XX_MISCCR) >> clkout->shift;
+	val = clkout->modify_misccr(0, 0) >> clkout->shift;
 	val >>= clkout->shift;
 	val &= clkout->mask;
 
@@ -76,7 +73,7 @@  static int s3c24xx_clkout_set_parent(struct clk_hw *hw, u8 index)
 {
 	struct s3c24xx_clkout *clkout = to_s3c24xx_clkout(hw);
 
-	s3c2410_modify_misccr((clkout->mask << clkout->shift),
+	clkout->modify_misccr((clkout->mask << clkout->shift),
 			      (index << clkout->shift));
 
 	return 0;
@@ -110,6 +107,7 @@  static struct clk_hw *s3c24xx_register_clkout(struct device *dev,
 	clkout->shift = shift;
 	clkout->mask = mask;
 	clkout->hw.init = &init;
+	clkout->modify_misccr = dev->platform_data;
 
 	ret = clk_hw_register(dev, &clkout->hw);
 	if (ret)