All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: Vasily Khoruzhick <anarsoul@gmail.com>,
	Yangtao Li <tiny.windzz@gmail.com>, Chen-Yu Tsai <wens@csie.org>,
	Samuel Holland <samuel@sholland.org>,
	Andre Przywara <andre.przywara@arm.com>
Cc: "Rafael J . Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Lukasz Luba <lukasz.luba@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Martin Botka <martin.botka@somainline.org>,
	Maksim Kiselev <bigunclemax@gmail.com>,
	Bob McChesney <bob@electricworry.net>,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v4 1/7] soc: sunxi: sram: export register 0 for THS on H616
Date: Wed, 14 Feb 2024 21:29:30 +0100	[thread overview]
Message-ID: <2172811.irdbgypaU6@jernej-laptop> (raw)
In-Reply-To: <20240209144221.3602382-2-andre.przywara@arm.com>

Dne petek, 09. februar 2024 ob 15:42:15 CET je Andre Przywara napisal(a):
> The Allwinner H616 SoC contains a mysterious bit at register offset 0x0
> in the SRAM control block. If bit 16 is set (the reset value), the
> temperature readings of the THS are way off, leading to reports about
> 200C, at normal ambient temperatures. Clearing this bits brings the
> reported values down to reasonable ranges.
> The BSP code clears this bit in firmware (U-Boot), and has an explicit
> comment about this, but offers no real explanation.
> 
> Since we should not rely on firmware settings, allow other code (the THS
> driver) to access this register, by exporting it through the already
> existing regmap. This mimics what we already do for the LDO control and
> the EMAC register.

Are you sure that this bit doesn't control actual SRAM region?

Best regards,
Jernej

> 
> Since this bit is in the very same register as the actual SRAM switch,
> we need to change the regmap lock to the SRAM lock. Fortunately regmap
> has provisions for that, so we just need to hook in there.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/soc/sunxi/sunxi_sram.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index 4458b2e0562b0..71cdd1b257eeb 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
> @@ -287,6 +287,7 @@ EXPORT_SYMBOL(sunxi_sram_release);
>  struct sunxi_sramc_variant {
>  	int num_emac_clocks;
>  	bool has_ldo_ctrl;
> +	bool has_ths_offset;
>  };
>  
>  static const struct sunxi_sramc_variant sun4i_a10_sramc_variant = {
> @@ -308,8 +309,10 @@ static const struct sunxi_sramc_variant sun50i_a64_sramc_variant = {
>  
>  static const struct sunxi_sramc_variant sun50i_h616_sramc_variant = {
>  	.num_emac_clocks = 2,
> +	.has_ths_offset = true,
>  };
>  
> +#define SUNXI_SRAM_THS_OFFSET_REG	0x0
>  #define SUNXI_SRAM_EMAC_CLOCK_REG	0x30
>  #define SUNXI_SYS_LDO_CTRL_REG		0x150
>  
> @@ -318,6 +321,8 @@ static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
>  {
>  	const struct sunxi_sramc_variant *variant = dev_get_drvdata(dev);
>  
> +	if (reg == SUNXI_SRAM_THS_OFFSET_REG && variant->has_ths_offset)
> +		return true;
>  	if (reg >= SUNXI_SRAM_EMAC_CLOCK_REG &&
>  	    reg <  SUNXI_SRAM_EMAC_CLOCK_REG + variant->num_emac_clocks * 4)
>  		return true;
> @@ -327,6 +332,21 @@ static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
>  	return false;
>  }
>  
> +
> +static void sunxi_sram_lock(void *_lock)
> +{
> +	spinlock_t *lock = _lock;
> +
> +	spin_lock(lock);
> +}
> +
> +static void sunxi_sram_unlock(void *_lock)
> +{
> +	spinlock_t *lock = _lock;
> +
> +	spin_unlock(lock);
> +}
> +
>  static struct regmap_config sunxi_sram_regmap_config = {
>  	.reg_bits       = 32,
>  	.val_bits       = 32,
> @@ -336,6 +356,9 @@ static struct regmap_config sunxi_sram_regmap_config = {
>  	/* other devices have no business accessing other registers */
>  	.readable_reg	= sunxi_sram_regmap_accessible_reg,
>  	.writeable_reg	= sunxi_sram_regmap_accessible_reg,
> +	.lock		= sunxi_sram_lock,
> +	.unlock		= sunxi_sram_unlock,
> +	.lock_arg	= &sram_lock,
>  };
>  
>  static int __init sunxi_sram_probe(struct platform_device *pdev)
> 





WARNING: multiple messages have this Message-ID (diff)
From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: Vasily Khoruzhick <anarsoul@gmail.com>,
	Yangtao Li <tiny.windzz@gmail.com>, Chen-Yu Tsai <wens@csie.org>,
	Samuel Holland <samuel@sholland.org>,
	Andre Przywara <andre.przywara@arm.com>
Cc: "Rafael J . Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Lukasz Luba <lukasz.luba@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Martin Botka <martin.botka@somainline.org>,
	Maksim Kiselev <bigunclemax@gmail.com>,
	Bob McChesney <bob@electricworry.net>,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v4 1/7] soc: sunxi: sram: export register 0 for THS on H616
Date: Wed, 14 Feb 2024 21:29:30 +0100	[thread overview]
Message-ID: <2172811.irdbgypaU6@jernej-laptop> (raw)
In-Reply-To: <20240209144221.3602382-2-andre.przywara@arm.com>

Dne petek, 09. februar 2024 ob 15:42:15 CET je Andre Przywara napisal(a):
> The Allwinner H616 SoC contains a mysterious bit at register offset 0x0
> in the SRAM control block. If bit 16 is set (the reset value), the
> temperature readings of the THS are way off, leading to reports about
> 200C, at normal ambient temperatures. Clearing this bits brings the
> reported values down to reasonable ranges.
> The BSP code clears this bit in firmware (U-Boot), and has an explicit
> comment about this, but offers no real explanation.
> 
> Since we should not rely on firmware settings, allow other code (the THS
> driver) to access this register, by exporting it through the already
> existing regmap. This mimics what we already do for the LDO control and
> the EMAC register.

Are you sure that this bit doesn't control actual SRAM region?

Best regards,
Jernej

> 
> Since this bit is in the very same register as the actual SRAM switch,
> we need to change the regmap lock to the SRAM lock. Fortunately regmap
> has provisions for that, so we just need to hook in there.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/soc/sunxi/sunxi_sram.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index 4458b2e0562b0..71cdd1b257eeb 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
> @@ -287,6 +287,7 @@ EXPORT_SYMBOL(sunxi_sram_release);
>  struct sunxi_sramc_variant {
>  	int num_emac_clocks;
>  	bool has_ldo_ctrl;
> +	bool has_ths_offset;
>  };
>  
>  static const struct sunxi_sramc_variant sun4i_a10_sramc_variant = {
> @@ -308,8 +309,10 @@ static const struct sunxi_sramc_variant sun50i_a64_sramc_variant = {
>  
>  static const struct sunxi_sramc_variant sun50i_h616_sramc_variant = {
>  	.num_emac_clocks = 2,
> +	.has_ths_offset = true,
>  };
>  
> +#define SUNXI_SRAM_THS_OFFSET_REG	0x0
>  #define SUNXI_SRAM_EMAC_CLOCK_REG	0x30
>  #define SUNXI_SYS_LDO_CTRL_REG		0x150
>  
> @@ -318,6 +321,8 @@ static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
>  {
>  	const struct sunxi_sramc_variant *variant = dev_get_drvdata(dev);
>  
> +	if (reg == SUNXI_SRAM_THS_OFFSET_REG && variant->has_ths_offset)
> +		return true;
>  	if (reg >= SUNXI_SRAM_EMAC_CLOCK_REG &&
>  	    reg <  SUNXI_SRAM_EMAC_CLOCK_REG + variant->num_emac_clocks * 4)
>  		return true;
> @@ -327,6 +332,21 @@ static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
>  	return false;
>  }
>  
> +
> +static void sunxi_sram_lock(void *_lock)
> +{
> +	spinlock_t *lock = _lock;
> +
> +	spin_lock(lock);
> +}
> +
> +static void sunxi_sram_unlock(void *_lock)
> +{
> +	spinlock_t *lock = _lock;
> +
> +	spin_unlock(lock);
> +}
> +
>  static struct regmap_config sunxi_sram_regmap_config = {
>  	.reg_bits       = 32,
>  	.val_bits       = 32,
> @@ -336,6 +356,9 @@ static struct regmap_config sunxi_sram_regmap_config = {
>  	/* other devices have no business accessing other registers */
>  	.readable_reg	= sunxi_sram_regmap_accessible_reg,
>  	.writeable_reg	= sunxi_sram_regmap_accessible_reg,
> +	.lock		= sunxi_sram_lock,
> +	.unlock		= sunxi_sram_unlock,
> +	.lock_arg	= &sram_lock,
>  };
>  
>  static int __init sunxi_sram_probe(struct platform_device *pdev)
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-02-14 20:29 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-09 14:42 [PATCH v4 0/7] add support for H616 thermal system Andre Przywara
2024-02-09 14:42 ` Andre Przywara
2024-02-09 14:42 ` [PATCH v4 1/7] soc: sunxi: sram: export register 0 for THS on H616 Andre Przywara
2024-02-09 14:42   ` Andre Przywara
2024-02-14 20:29   ` Jernej Škrabec [this message]
2024-02-14 20:29     ` Jernej Škrabec
2024-02-15  1:28     ` Andre Przywara
2024-02-15  1:28       ` Andre Przywara
2024-02-15 21:18       ` Jernej Škrabec
2024-02-15 21:18         ` Jernej Škrabec
2024-02-09 14:42 ` [PATCH v4 2/7] dt-bindings: thermal: sun8i: Add H616 THS controller Andre Przywara
2024-02-09 14:42   ` Andre Przywara
2024-02-11 16:31   ` Krzysztof Kozlowski
2024-02-11 16:31     ` Krzysztof Kozlowski
2024-02-09 14:42 ` [PATCH v4 3/7] thermal: sun8i: explain unknown H6 register value Andre Przywara
2024-02-09 14:42   ` Andre Przywara
2024-02-14 20:31   ` Jernej Škrabec
2024-02-14 20:31     ` Jernej Škrabec
2024-02-09 14:42 ` [PATCH v4 4/7] thermal: sun8i: extend H6 calibration to support 4 sensors Andre Przywara
2024-02-09 14:42   ` Andre Przywara
2024-02-14 20:35   ` Jernej Škrabec
2024-02-14 20:35     ` Jernej Škrabec
2024-02-09 14:42 ` [PATCH v4 5/7] thermal: sun8i: add SRAM register access code Andre Przywara
2024-02-09 14:42   ` Andre Przywara
2024-02-15 21:24   ` Jernej Škrabec
2024-02-15 21:24     ` Jernej Škrabec
2024-02-09 14:42 ` [PATCH v4 6/7] thermal: sun8i: add support for H616 THS controller Andre Przywara
2024-02-09 14:42   ` Andre Przywara
2024-02-09 14:42 ` [PATCH v4 7/7] arm64: dts: allwinner: h616: Add thermal sensor and zones Andre Przywara
2024-02-09 14:42   ` Andre Przywara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2172811.irdbgypaU6@jernej-laptop \
    --to=jernej.skrabec@gmail.com \
    --cc=anarsoul@gmail.com \
    --cc=andre.przywara@arm.com \
    --cc=bigunclemax@gmail.com \
    --cc=bob@electricworry.net \
    --cc=conor+dt@kernel.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=lukasz.luba@arm.com \
    --cc=martin.botka@somainline.org \
    --cc=rafael@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=samuel@sholland.org \
    --cc=tiny.windzz@gmail.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.