Re: [PATCH] drm/msm/adreno: De-spaghettify the use of memory barriers

2024-05-16 Thread Andrew Halaney
On Thu, May 16, 2024 at 08:20:05PM GMT, Akhil P Oommen wrote:
> On Thu, May 16, 2024 at 08:15:34AM -0500, Andrew Halaney wrote:
> > On Wed, May 15, 2024 at 12:08:49AM GMT, Akhil P Oommen wrote:
> > > On Wed, May 08, 2024 at 07:46:31PM +0200, Konrad Dybcio wrote:
> > > > Memory barriers help ensure instruction ordering, NOT time and order
> > > > of actual write arrival at other observers (e.g. memory-mapped IP).
> > > > On architectures employing weak memory ordering, the latter can be a
> > > > giant pain point, and it has been as part of this driver.
> > > > 
> > > > Moreover, the gpu_/gmu_ accessors already use non-relaxed versions of
> > > > readl/writel, which include r/w (respectively) barriers.
> > > > 
> > > > Replace the barriers with a readback that ensures the previous writes
> > > > have exited the write buffer (as the CPU must flush the write to the
> > > > register it's trying to read back) and subsequently remove the hack
> > > > introduced in commit b77532803d11 ("drm/msm/a6xx: Poll for GBIF unhalt
> > > > status in hw_init").
> > 
> > For what its worth, I've been eyeing (but haven't tested) sending some
> > patches to clean up dsi_phy_write_udelay/ndelay(). There's no ordering
> > guarantee between a writel() and a delay(), so the expected "write then
> > delay" sequence might not be happening.. you need to write, read, delay.
> > 
> > memory-barriers.txt:
> > 
> > 5. A readX() by a CPU thread from the peripheral will complete before
> >any subsequent delay() loop can begin execution on the same thread.
> >This ensures that two MMIO register writes by the CPU to a peripheral
> >will arrive at least 1us apart if the first write is immediately read
> >back with readX() and udelay(1) is called prior to the second
> >writeX():
> > 
> > writel(42, DEVICE_REGISTER_0); // Arrives at the device...
> > readl(DEVICE_REGISTER_0);
> > udelay(1);
> > writel(42, DEVICE_REGISTER_1); // ...at least 1us before this.
> 
> Yes, udelay orders only with readl(). I saw a patch from Will Deacon
> which fixes this for arm64 few years back:
> https://lore.kernel.org/all/1543251228-30001-1-git-send-email-will.dea...@arm.com/T/
> 
> But this is needed only when you write io and do cpuside wait , not when
> you poll io to check status.

Sure, I'm just highlighting the bug in dsi_phy_write_*delay() functions
here, which match the udelay case you mention.

> 
> > 
> > > > 
> > > > Fixes: b77532803d11 ("drm/msm/a6xx: Poll for GBIF unhalt status in 
> > > > hw_init")
> > > > Signed-off-by: Konrad Dybcio 
> > > > ---
> > > >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c |  5 ++---
> > > >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 14 --
> > > >  2 files changed, 6 insertions(+), 13 deletions(-)
> > > 
> > > I prefer this version compared to the v2. A helper routine is
> > > unnecessary here because:
> > > 1. there are very few scenarios where we have to read back the same
> > > register.
> > > 2. we may accidently readback a write only register.
> > > 
> > > > 
> > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
> > > > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > > index 0e3dfd4c2bc8..4135a53b55a7 100644
> > > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > > @@ -466,9 +466,8 @@ static int a6xx_rpmh_start(struct a6xx_gmu *gmu)
> > > > int ret;
> > > > u32 val;
> > > >  
> > > > -   gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 1 << 1);
> > > > -   /* Wait for the register to finish posting */
> > > > -   wmb();
> > > > +   gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, BIT(1));
> > > > +   gmu_read(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ);
> > > 
> > > This is unnecessary because we are polling on a register on the same port 
> > > below. But I think we
> > > can replace "wmb()" above with "mb()" to avoid reordering between read
> > > and write IO instructions.
> > 
> > If I understand correctly, you don't need any memory barrier.
> > writel()/readl()'s are ordered to the same endpoint. That goes for all
> > the reordering/barrier comments mentioned be

Re: [PATCH] drm/msm/adreno: De-spaghettify the use of memory barriers

2024-05-16 Thread Andrew Halaney
On Wed, May 15, 2024 at 12:08:49AM GMT, Akhil P Oommen wrote:
> On Wed, May 08, 2024 at 07:46:31PM +0200, Konrad Dybcio wrote:
> > Memory barriers help ensure instruction ordering, NOT time and order
> > of actual write arrival at other observers (e.g. memory-mapped IP).
> > On architectures employing weak memory ordering, the latter can be a
> > giant pain point, and it has been as part of this driver.
> > 
> > Moreover, the gpu_/gmu_ accessors already use non-relaxed versions of
> > readl/writel, which include r/w (respectively) barriers.
> > 
> > Replace the barriers with a readback that ensures the previous writes
> > have exited the write buffer (as the CPU must flush the write to the
> > register it's trying to read back) and subsequently remove the hack
> > introduced in commit b77532803d11 ("drm/msm/a6xx: Poll for GBIF unhalt
> > status in hw_init").

For what its worth, I've been eyeing (but haven't tested) sending some
patches to clean up dsi_phy_write_udelay/ndelay(). There's no ordering
guarantee between a writel() and a delay(), so the expected "write then
delay" sequence might not be happening.. you need to write, read, delay.

memory-barriers.txt:

5. A readX() by a CPU thread from the peripheral will complete before
   any subsequent delay() loop can begin execution on the same thread.
   This ensures that two MMIO register writes by the CPU to a peripheral
   will arrive at least 1us apart if the first write is immediately read
   back with readX() and udelay(1) is called prior to the second
   writeX():

writel(42, DEVICE_REGISTER_0); // Arrives at the device...
readl(DEVICE_REGISTER_0);
udelay(1);
writel(42, DEVICE_REGISTER_1); // ...at least 1us before this.

> > 
> > Fixes: b77532803d11 ("drm/msm/a6xx: Poll for GBIF unhalt status in hw_init")
> > Signed-off-by: Konrad Dybcio 
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c |  5 ++---
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 14 --
> >  2 files changed, 6 insertions(+), 13 deletions(-)
> 
> I prefer this version compared to the v2. A helper routine is
> unnecessary here because:
> 1. there are very few scenarios where we have to read back the same
> register.
> 2. we may accidently readback a write only register.
> 
> > 
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
> > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index 0e3dfd4c2bc8..4135a53b55a7 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -466,9 +466,8 @@ static int a6xx_rpmh_start(struct a6xx_gmu *gmu)
> > int ret;
> > u32 val;
> >  
> > -   gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 1 << 1);
> > -   /* Wait for the register to finish posting */
> > -   wmb();
> > +   gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, BIT(1));
> > +   gmu_read(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ);
> 
> This is unnecessary because we are polling on a register on the same port 
> below. But I think we
> can replace "wmb()" above with "mb()" to avoid reordering between read
> and write IO instructions.

If I understand correctly, you don't need any memory barrier.
writel()/readl()'s are ordered to the same endpoint. That goes for all
the reordering/barrier comments mentioned below too.

device-io.rst:

The read and write functions are defined to be ordered. That is the
compiler is not permitted to reorder the I/O sequence. When the ordering
can be compiler optimised, you can use __readb() and friends to
indicate the relaxed ordering. Use this with care.

memory-barriers.txt:

 (*) readX(), writeX():

The readX() and writeX() MMIO accessors take a pointer to the
peripheral being accessed as an __iomem * parameter. For pointers
mapped with the default I/O attributes (e.g. those returned by
ioremap()), the ordering guarantees are as follows:

1. All readX() and writeX() accesses to the same peripheral are 
ordered
   with respect to each other. This ensures that MMIO register 
accesses
   by the same CPU thread to a particular device will arrive in 
program
   order.


> 
> >  
> > ret = gmu_poll_timeout(gmu, REG_A6XX_GMU_RSCC_CONTROL_ACK, val,
> > val & (1 << 1), 100, 1);
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index 973872ad0474..0acbc38b8e70 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -1713,22 +1713,16 @@ static int hw_init(struct msm_gpu *gpu)
> > }
> >  
> > /* Clear GBIF halt in case GX domain was not collapsed */
> > +   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
> 
> We need a full barrier here to avoid reordering. Also, lets add a
> comment about why we are doing this odd looking sequence.
> 
> > +   gpu_read(gpu, 

Re: [PATCH] drm/msm: Drop msm_read/writel

2024-04-10 Thread Andrew Halaney
On Wed, Apr 10, 2024 at 11:52:52PM +0200, Konrad Dybcio wrote:
> Totally useless.
> 
> Signed-off-by: Konrad Dybcio 

A few more words in the description mentioning this just wraps readl/writel
with no, but that's a minor nit and is easy to find when you finally see
the removal in the end of the diff.

Reviewed-by: Andrew Halaney 

> ---
> only compile-tested
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c   |  2 +-
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.h   | 12 ++--
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.h   |  4 ++--
>  drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |  4 ++--
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h|  4 ++--
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h|  4 ++--
>  drivers/gpu/drm/msm/dsi/dsi_host.c  | 10 +-
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h   |  8 
>  drivers/gpu/drm/msm/hdmi/hdmi.h | 10 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c|  6 +++---
>  drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c|  4 ++--
>  drivers/gpu/drm/msm/msm_drv.h   |  7 ++-
>  drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
>  13 files changed, 42 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 8bea8ef26f77..0e3dfd4c2bc8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -507,7 +507,7 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
>  
>  static inline void pdc_write(void __iomem *ptr, u32 offset, u32 value)
>  {
> - msm_writel(value, ptr + (offset << 2));
> + writel(value, ptr + (offset << 2));
>  }
>  
>  static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index 592b296aab22..94b6c5cab6f4 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -103,12 +103,12 @@ struct a6xx_gmu {
>  
>  static inline u32 gmu_read(struct a6xx_gmu *gmu, u32 offset)
>  {
> - return msm_readl(gmu->mmio + (offset << 2));
> + return readl(gmu->mmio + (offset << 2));
>  }
>  
>  static inline void gmu_write(struct a6xx_gmu *gmu, u32 offset, u32 value)
>  {
> - msm_writel(value, gmu->mmio + (offset << 2));
> + writel(value, gmu->mmio + (offset << 2));
>  }
>  
>  static inline void
> @@ -131,8 +131,8 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32 
> lo, u32 hi)
>  {
>   u64 val;
>  
> - val = (u64) msm_readl(gmu->mmio + (lo << 2));
> - val |= ((u64) msm_readl(gmu->mmio + (hi << 2)) << 32);
> + val = (u64) readl(gmu->mmio + (lo << 2));
> + val |= ((u64) readl(gmu->mmio + (hi << 2)) << 32);
>  
>   return val;
>  }
> @@ -143,12 +143,12 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32 
> lo, u32 hi)
>  
>  static inline u32 gmu_read_rscc(struct a6xx_gmu *gmu, u32 offset)
>  {
> - return msm_readl(gmu->rscc + (offset << 2));
> + return readl(gmu->rscc + (offset << 2));
>  }
>  
>  static inline void gmu_write_rscc(struct a6xx_gmu *gmu, u32 offset, u32 
> value)
>  {
> - msm_writel(value, gmu->rscc + (offset << 2));
> + writel(value, gmu->rscc + (offset << 2));
>  }
>  
>  #define gmu_poll_timeout_rscc(gmu, addr, val, cond, interval, timeout) \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> index 34822b080759..8917032b7515 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> @@ -69,12 +69,12 @@ static inline void a6xx_llc_rmw(struct a6xx_gpu 
> *a6xx_gpu, u32 reg, u32 mask, u3
>  
>  static inline u32 a6xx_llc_read(struct a6xx_gpu *a6xx_gpu, u32 reg)
>  {
> - return msm_readl(a6xx_gpu->llc_mmio + (reg << 2));
> + return readl(a6xx_gpu->llc_mmio + (reg << 2));
>  }
>  
>  static inline void a6xx_llc_write(struct a6xx_gpu *a6xx_gpu, u32 reg, u32 
> value)
>  {
> - msm_writel(value, a6xx_gpu->llc_mmio + (reg << 2));
> + writel(value, a6xx_gpu->llc_mmio + (reg << 2));
>  }
>  
>  #define shadowptr(_a6xx_gpu, _ring) ((_a6xx_gpu)->shadow_iova + \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> index a847a0f7a73c..83d7ee01c944 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> @@ -192,10 +192,10 @@ static int debugbus_r

[PATCH] drm/tidss: Use dev_err_probe() over dev_dbg() when failing to probe the port

2024-02-28 Thread Andrew Halaney
This gets logged out to /sys/kernel/debug/devices_deferred in the
-EPROBE_DEFER case and as an error otherwise. The message here provides
useful information to the user when troubleshooting why their display is
not working in either case, so let's make it output appropriately.

Signed-off-by: Andrew Halaney 
---
There's definitely more spots in this driver that could be upgraded from
dev_dbg() to something more appropriate, but this one burned me today so
I thought I'd send a patch for it specifically before I forget.
---
 drivers/gpu/drm/tidss/tidss_kms.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_kms.c 
b/drivers/gpu/drm/tidss/tidss_kms.c
index a0e494c806a96..f371518f86971 100644
--- a/drivers/gpu/drm/tidss/tidss_kms.c
+++ b/drivers/gpu/drm/tidss/tidss_kms.c
@@ -135,8 +135,7 @@ static int tidss_dispc_modeset_init(struct tidss_device 
*tidss)
dev_dbg(dev, "no panel/bridge for port %d\n", i);
continue;
} else if (ret) {
-   dev_dbg(dev, "port %d probe returned %d\n", i, ret);
-   return ret;
+   return dev_err_probe(dev, ret, "port %d probe 
failed\n", i);
}
 
if (panel) {

---
base-commit: 22ba90670a51a18c6b36d285fddf92b9887c0bc3
change-id: 20240228-tidss-dev-err-probe-fa61fb057029

Best regards,
-- 
Andrew Halaney 



Re: [PATCH 3/8] clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support

2024-02-19 Thread Andrew Halaney
On Mon, Feb 19, 2024 at 02:35:48PM +0100, Konrad Dybcio wrote:
> Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL")
> introduced an entry to the alpha offsets array, but diving into QCM2290
> downstream and some documentation, it turned out that the name Huayra
> apparently has been used quite liberally across many chips, even with
> noticeably different hardware.
> 
> Introduce another set of offsets and a new configure function for the
> Huayra PLL found on QCM2290. This is required e.g. for the consumers
> of GPUCC_PLL0 to properly start.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  drivers/clk/qcom/clk-alpha-pll.c | 45 
> 
>  drivers/clk/qcom/clk-alpha-pll.h |  3 +++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/clk/qcom/clk-alpha-pll.c 
> b/drivers/clk/qcom/clk-alpha-pll.c
> index 8a412ef47e16..61b5abd13782 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.c
> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> @@ -244,6 +244,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
>   [PLL_OFF_OPMODE] = 0x30,
>   [PLL_OFF_STATUS] = 0x3c,
>   },
> + [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] =  {
> + [PLL_OFF_L_VAL] = 0x04,
> + [PLL_OFF_ALPHA_VAL] = 0x08,
> + [PLL_OFF_USER_CTL] = 0x0c,
> + [PLL_OFF_CONFIG_CTL] = 0x10,
> + [PLL_OFF_CONFIG_CTL_U] = 0x14,
> + [PLL_OFF_CONFIG_CTL_U1] = 0x18,
> + [PLL_OFF_TEST_CTL] = 0x1c,
> + [PLL_OFF_TEST_CTL_U] = 0x20,
> + [PLL_OFF_TEST_CTL_U1] = 0x24,
> + [PLL_OFF_OPMODE] = 0x28,
> + [PLL_OFF_STATUS] = 0x38,
> + },
>  };
>  EXPORT_SYMBOL_GPL(clk_alpha_pll_regs);
>  
> @@ -779,6 +792,38 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, 
> unsigned long rate,
>   return clamp(rate, min_freq, max_freq);
>  }
>  
> +void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap 
> *regmap,
> +const struct alpha_pll_config *config)
> +{
> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), 
> config->config_ctl_val);
> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), 
> config->config_ctl_hi_val);
> + clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), 
> config->config_ctl_hi1_val);
> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), 
> config->test_ctl_val);
> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), 
> config->test_ctl_hi_val);
> + clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), 
> config->test_ctl_hi1_val);
> + clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
> + clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
> + clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), 
> config->user_ctl_val);
> +
> + /* Set PLL_BYPASSNL */
> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL);
> +
> + /* Wait 5 us between setting BYPASS and deasserting reset */
> + mb();
> + udelay(5);
> +
> + /* Take PLL out from reset state */
> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
> +
> + /* Wait 50us for PLL_LOCK_DET bit to go high */
> + mb();
> + usleep_range(50, 55);

I *think* you'd want to use a read to ensure your write goes through
prior to your sleep... from memory-barriers.txt:

5. A readX() by a CPU thread from the peripheral will complete before
   any subsequent delay() loop can begin execution on the same thread.
   This ensures that two MMIO register writes by the CPU to a peripheral
   will arrive at least 1us apart if the first write is immediately read
   back with readX() and udelay(1) is called prior to the second
   writeX():

writel(42, DEVICE_REGISTER_0); // Arrives at the device...
readl(DEVICE_REGISTER_0);
udelay(1);
writel(42, DEVICE_REGISTER_1); // ...at least 1us before this.

also https://youtu.be/i6DayghhA8Q?si=7lp0be35q1HRmlnV=1677
for more references on this topic.


> +
> + /* Enable PLL output */
> + regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, PLL_OUTCTRL);
> +}
> +EXPORT_SYMBOL(clk_huayra_2290_pll_configure);
> +
>  static unsigned long
>  alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a)
>  {
> diff --git a/drivers/clk/qcom/clk-alpha-pll.h 
> b/drivers/clk/qcom/clk-alpha-pll.h
> index fb6d50263bb9..91d3d6f19eae 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.h
> +++ b/drivers/clk/qcom/clk-alpha-pll.h
> @@ -29,6 +29,7 @@ enum {
>   CLK_ALPHA_PLL_TYPE_BRAMMO_EVO,
>   CLK_ALPHA_PLL_TYPE_STROMER,
>   CLK_ALPHA_PLL_TYPE_STROMER_PLUS,
> + CLK_ALPHA_PLL_TYPE_HUAYRA_2290,
>   CLK_ALPHA_PLL_TYPE_MAX,
>  };
>  
> @@ -191,6 +192,8 @@ extern const struct clk_ops clk_alpha_pll_rivian_evo_ops;
>  
>  void clk_alpha_pll_configure(struct 

Re: [PATCH] fbdev/simplefb: change loglevel when the power domains cannot be parsed

2023-12-12 Thread Andrew Halaney
On Tue, Dec 12, 2023 at 02:57:54PM -0500, Brian Masney wrote:
> When the power domains cannot be parsed, the message is incorrectly
> logged as an info message. Let's change this to an error since an error
> is returned.
> 
> Fixes: 92a511a568e4 ("fbdev/simplefb: Add support for generic power-domains")
> Signed-off-by: Brian Masney 

Acked-by: Andrew Halaney 

> ---
>  drivers/video/fbdev/simplefb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 6f58ee276ad1..028a56525047 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -470,7 +470,7 @@ static int simplefb_attach_genpds(struct simplefb_par 
> *par,
>   if (err == -ENOENT)
>   return 0;
>  
> - dev_info(dev, "failed to parse power-domains: %d\n", err);
> + dev_err(dev, "failed to parse power-domains: %d\n", err);
>   return err;
>   }
>  
> -- 
> 2.43.0
>