[PATCH v3] drivers/clocksource/mediatek: Ack and disable interrupts on suspend

2021-04-11 Thread Evan Benn 
Interrupts are disabled during suspend before this driver disables its
timers. ARM trusted firmware will abort suspend if the timer irq is
pending, so ack and disable the timer interrupt during suspend.

Signed-off-by: Evan Benn 
---

Changes in v3:
Move the ACK from the shutdown to the suspend function.

Changes in v2:
Remove the patch that splits the drivers into 2 files.

 drivers/clocksource/timer-mediatek.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/clocksource/timer-mediatek.c 
b/drivers/clocksource/timer-mediatek.c
index 9318edcd8963..1ae8fee639bf 100644
--- a/drivers/clocksource/timer-mediatek.c
+++ b/drivers/clocksource/timer-mediatek.c
@@ -241,6 +241,27 @@ static void mtk_gpt_enable_irq(struct timer_of *to, u8 
timer)
   timer_of_base(to) + GPT_IRQ_EN_REG);
 }
 
+static void mtk_gpt_resume(struct clock_event_device *clk)
+{
+   struct timer_of *to = to_timer_of(clk);
+
+   mtk_gpt_enable_irq(to, TIMER_CLK_EVT);
+}
+
+static void mtk_gpt_suspend(struct clock_event_device *clk)
+{
+   struct timer_of *to = to_timer_of(clk);
+
+   /* Disable all interrupts */
+   writel(0x0, timer_of_base(to) + GPT_IRQ_EN_REG);
+
+   /* This is called with interrupts disabled,
+* so we need to ack any interrupt that is pending
+* Or for example ATF will prevent a suspend from completing.
+*/
+   writel(0x3f, timer_of_base(to) + GPT_IRQ_ACK_REG);
+}
+
 static struct timer_of to = {
.flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
 
@@ -286,6 +307,8 @@ static int __init mtk_gpt_init(struct device_node *node)
to.clkevt.set_state_oneshot = mtk_gpt_clkevt_shutdown;
to.clkevt.tick_resume = mtk_gpt_clkevt_shutdown;
to.clkevt.set_next_event = mtk_gpt_clkevt_next_event;
+   to.clkevt.suspend = mtk_gpt_suspend;
+   to.clkevt.resume = mtk_gpt_resume;
to.of_irq.handler = mtk_gpt_interrupt;
 
ret = timer_of_init(node, );
-- 
2.31.1.295.g9ea45b61b8-goog

Re: [PATCH v2] drivers/clocksource/mediatek: Ack and disable interrupts on shutdown

2021-04-05 Thread Evan Benn 
On Sat, Apr 3, 2021 at 1:15 AM Daniel Lezcano  wrote:
> >
> > That said I am happy to upload that version if people think it is better.
>
> IMO, it is not in the normal flow to disable/enable the interrupts.

Hi Daniel, no problem.

I will upload the suspend/resume version. Please note the similar fix
to the other timer in this file:

https://patchwork.kernel.org/project/linux-mediatek/patch/1614670085-26229-2-git-send-email-fengquan.c...@mediatek.com/

Is doing the same ack/disable.

>
> Does this timer belong to an always-on power domain ?
>

I'm not sure sorry, the datasheet does not seem to say. Maybe mediatek
can answer.

Re: [PATCH v2] drivers/clocksource/mediatek: Ack and disable interrupts on shutdown

2021-03-26 Thread Evan Benn 
Hi Daniel,

That is a good point, and I did try that at first and it works fine. I
uploaded this version because the suspend/resume callbacks were
undocumented and mostly not used by other clocksource drivers. I
thought a smaller diff might be preferable.
I also thought it would be better to shut off the interrupt as soon as
it is not needed, avoiding any other potential bugs, instead of just
fixing the one we know about with suspend. I'm not sure how the other
driver / timer disable flows are intended to work for example
(shutdown, detach, etc).

That said I am happy to upload that version if people think it is better.

https://elixir.bootlin.com/linux/latest/source/include/linux/clockchips.h#L120


On Thu, 25 Mar 2021 at 19:10, Daniel Lezcano  wrote:
>
> On 25/03/2021 02:35, Evan Benn wrote:
> > set_state_shutdown is called during system suspend after interrupts have
> > been disabled. If the timer has fired in the meantime, there will be
> > a pending IRQ. So we ack that now and disable the timer. Without this
> > ARM trusted firmware will abort the suspend due to the pending
> > interrupt.
> >
> > Now always disable the IRQ in state transitions, and re-enable in
> > set_periodic and next_event.
>
> Why not put add the suspend/resume callbacks and put there the specific
> code and let the irq untouched in the normal flow ?
>
> > Signed-off-by: Evan Benn 
> > ---
> >
> > Changes in v2:
> > Remove the patch that splits the drivers into 2 files.
> >
> >  drivers/clocksource/timer-mediatek.c | 49 +---
> >  1 file changed, 30 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-mediatek.c 
> > b/drivers/clocksource/timer-mediatek.c
> > index 9318edcd8963..fba2f9494d90 100644
> > --- a/drivers/clocksource/timer-mediatek.c
> > +++ b/drivers/clocksource/timer-mediatek.c
> > @@ -132,13 +132,33 @@ static u64 notrace mtk_gpt_read_sched_clock(void)
> >   return readl_relaxed(gpt_sched_reg);
> >  }
> >
> > +static void mtk_gpt_disable_ack_interrupts(struct timer_of *to, u8 timer)
> > +{
> > + u32 val;
> > +
> > + /* Disable interrupts */
> > + val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> > + writel(val & ~GPT_IRQ_ENABLE(timer), timer_of_base(to) +
> > +GPT_IRQ_EN_REG);
> > +
> > + /* Ack interrupts */
> > + writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
> > +}
> > +
> >  static void mtk_gpt_clkevt_time_stop(struct timer_of *to, u8 timer)
> >  {
> >   u32 val;
> >
> > + /* Disable timer */
> >   val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
> >   writel(val & ~GPT_CTRL_ENABLE, timer_of_base(to) +
> >  GPT_CTRL_REG(timer));
> > +
> > + /* This may be called with interrupts disabled,
> > +  * so we need to ack any interrupt that is pending
> > +  * Or for example ATF will prevent a suspend from completing.
> > +  */
> > + mtk_gpt_disable_ack_interrupts(to, timer);
> >  }
> >
> >  static void mtk_gpt_clkevt_time_setup(struct timer_of *to,
> > @@ -152,8 +172,10 @@ static void mtk_gpt_clkevt_time_start(struct timer_of 
> > *to,
> >  {
> >   u32 val;
> >
> > - /* Acknowledge interrupt */
> > - writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
> > + /* Enable interrupts */
> > + val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> > + writel(val | GPT_IRQ_ENABLE(timer),
> > +timer_of_base(to) + GPT_IRQ_EN_REG);
> >
> >   val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
> >
> > @@ -226,21 +248,6 @@ __init mtk_gpt_setup(struct timer_of *to, u8 timer, u8 
> > option)
> >  timer_of_base(to) + GPT_CTRL_REG(timer));
> >  }
> >
> > -static void mtk_gpt_enable_irq(struct timer_of *to, u8 timer)
> > -{
> > - u32 val;
> > -
> > - /* Disable all interrupts */
> > - writel(0x0, timer_of_base(to) + GPT_IRQ_EN_REG);
> > -
> > - /* Acknowledge all spurious pending interrupts */
> > - writel(0x3f, timer_of_base(to) + GPT_IRQ_ACK_REG);
> > -
> > - val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> > - writel(val | GPT_IRQ_ENABLE(timer),
> > -timer_of_base(to) + GPT_IRQ_EN_REG);
> > -}
> > -
> >  static struct timer_of to = {
> >   .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> >
> > @@ -292,6 +299,12 @@ static int __init mtk_gpt_init(struct device_node 
> > *node

[PATCH v2] drivers/clocksource/mediatek: Ack and disable interrupts on shutdown

2021-03-24 Thread Evan Benn 
set_state_shutdown is called during system suspend after interrupts have
been disabled. If the timer has fired in the meantime, there will be
a pending IRQ. So we ack that now and disable the timer. Without this
ARM trusted firmware will abort the suspend due to the pending
interrupt.

Now always disable the IRQ in state transitions, and re-enable in
set_periodic and next_event.

Signed-off-by: Evan Benn 
---

Changes in v2:
Remove the patch that splits the drivers into 2 files.

 drivers/clocksource/timer-mediatek.c | 49 +---
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/clocksource/timer-mediatek.c 
b/drivers/clocksource/timer-mediatek.c
index 9318edcd8963..fba2f9494d90 100644
--- a/drivers/clocksource/timer-mediatek.c
+++ b/drivers/clocksource/timer-mediatek.c
@@ -132,13 +132,33 @@ static u64 notrace mtk_gpt_read_sched_clock(void)
return readl_relaxed(gpt_sched_reg);
 }
 
+static void mtk_gpt_disable_ack_interrupts(struct timer_of *to, u8 timer)
+{
+   u32 val;
+
+   /* Disable interrupts */
+   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
+   writel(val & ~GPT_IRQ_ENABLE(timer), timer_of_base(to) +
+  GPT_IRQ_EN_REG);
+
+   /* Ack interrupts */
+   writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
+}
+
 static void mtk_gpt_clkevt_time_stop(struct timer_of *to, u8 timer)
 {
u32 val;
 
+   /* Disable timer */
val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
writel(val & ~GPT_CTRL_ENABLE, timer_of_base(to) +
   GPT_CTRL_REG(timer));
+
+   /* This may be called with interrupts disabled,
+* so we need to ack any interrupt that is pending
+* Or for example ATF will prevent a suspend from completing.
+*/
+   mtk_gpt_disable_ack_interrupts(to, timer);
 }
 
 static void mtk_gpt_clkevt_time_setup(struct timer_of *to,
@@ -152,8 +172,10 @@ static void mtk_gpt_clkevt_time_start(struct timer_of *to,
 {
u32 val;
 
-   /* Acknowledge interrupt */
-   writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
+   /* Enable interrupts */
+   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
+   writel(val | GPT_IRQ_ENABLE(timer),
+  timer_of_base(to) + GPT_IRQ_EN_REG);
 
val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
 
@@ -226,21 +248,6 @@ __init mtk_gpt_setup(struct timer_of *to, u8 timer, u8 
option)
   timer_of_base(to) + GPT_CTRL_REG(timer));
 }
 
-static void mtk_gpt_enable_irq(struct timer_of *to, u8 timer)
-{
-   u32 val;
-
-   /* Disable all interrupts */
-   writel(0x0, timer_of_base(to) + GPT_IRQ_EN_REG);
-
-   /* Acknowledge all spurious pending interrupts */
-   writel(0x3f, timer_of_base(to) + GPT_IRQ_ACK_REG);
-
-   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
-   writel(val | GPT_IRQ_ENABLE(timer),
-  timer_of_base(to) + GPT_IRQ_EN_REG);
-}
-
 static struct timer_of to = {
.flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
 
@@ -292,6 +299,12 @@ static int __init mtk_gpt_init(struct device_node *node)
if (ret)
return ret;
 
+   /* In case the firmware left the interrupts enabled
+* disable and ack those now
+*/
+   mtk_gpt_disable_ack_interrupts(, TIMER_CLK_SRC);
+   mtk_gpt_disable_ack_interrupts(, TIMER_CLK_EVT);
+
/* Configure clock source */
mtk_gpt_setup(, TIMER_CLK_SRC, GPT_CTRL_OP_FREERUN);
clocksource_mmio_init(timer_of_base() + GPT_CNT_REG(TIMER_CLK_SRC),
@@ -305,8 +318,6 @@ static int __init mtk_gpt_init(struct device_node *node)
clockevents_config_and_register(, timer_of_rate(),
TIMER_SYNC_TICKS, 0x);
 
-   mtk_gpt_enable_irq(, TIMER_CLK_EVT);
-
return 0;
 }
 TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
-- 
2.31.0.291.g576ba9dcdaf-goog

Re: [PATCH 1/2] drivers/clocksource/mediatek: Split mediatek drivers into 2 files

2021-03-23 Thread Evan Benn 
On Mon, Mar 22, 2021 at 10:19 PM Daniel Lezcano
 wrote:
>
>
> Hi Evan,
>
> On 18/03/2021 06:04, Evan Benn wrote:
> > mtk_gpt and mtk_syst drivers for mt6577 and mt6765 devices were not
> > sharing any code. So split them into separate files.
>
> For the sake of consistency, keeping all in one is better.

Thanks Daniel,

I think you are right.
The other patch does apply cleanly without this one, so please just
ignore this patch.

>
> Thanks
>   -- Daniel
>
> > Signed-off-by: Evan Benn 
> > ---
> >
> >  arch/arm/mach-mediatek/Kconfig|   3 +-
> >  arch/arm64/Kconfig.platforms  |   3 +-
> >  drivers/clocksource/Kconfig   |  13 +-
> >  drivers/clocksource/Makefile  |   3 +-
> >  ...mer-mediatek.c => timer-mediatek-mt6577.c} | 100 -
> >  drivers/clocksource/timer-mediatek-mt6765.c   | 135 ++
> >  6 files changed, 151 insertions(+), 106 deletions(-)
> >  rename drivers/clocksource/{timer-mediatek.c => timer-mediatek-mt6577.c} 
> > (69%)
> >  create mode 100644 drivers/clocksource/timer-mediatek-mt6765.c
> >
> > diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
> > index 9e0f592d87d8..8686f992c4b6 100644
> > --- a/arch/arm/mach-mediatek/Kconfig
> > +++ b/arch/arm/mach-mediatek/Kconfig
> > @@ -4,7 +4,8 @@ menuconfig ARCH_MEDIATEK
> >   depends on ARCH_MULTI_V7
> >   select ARM_GIC
> >   select PINCTRL
> > - select MTK_TIMER
> > + select TIMER_MEDIATEK_MT6577
> > + select TIMER_MEDIATEK_MT6765
> >   select MFD_SYSCON
> >   help
> > Support for Mediatek MT65xx & MT81xx SoCs
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index cdfd5fed457f..d4752375ab0b 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -161,7 +161,8 @@ config ARCH_MEDIATEK
> >   bool "MediaTek SoC Family"
> >   select ARM_GIC
> >   select PINCTRL
> > - select MTK_TIMER
> > + select TIMER_MEDIATEK_MT6577
> > + select TIMER_MEDIATEK_MT6765
> >   help
> > This enables support for MediaTek MT27xx, MT65xx, MT76xx
> > & MT81xx ARMv8 SoCs
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 39aa21d01e05..d697c799284e 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -438,13 +438,20 @@ config OXNAS_RPS_TIMER
> >  config SYS_SUPPORTS_SH_CMT
> >   bool
> >
> > -config MTK_TIMER
> > - bool "Mediatek timer driver" if COMPILE_TEST
> > +config TIMER_MEDIATEK_MT6577
> > + bool "Mediatek mt6577 timer driver" if COMPILE_TEST
> >   depends on HAS_IOMEM
> >   select TIMER_OF
> >   select CLKSRC_MMIO
> >   help
> > -   Support for Mediatek timer driver.
> > +   Enables clocksource and clockevent driver for Mediatek mt6577 timer.
> > +
> > +config TIMER_MEDIATEK_MT6765
> > + bool "Mediatek mt6765 timer driver" if COMPILE_TEST
> > + depends on HAS_IOMEM
> > + select TIMER_OF
> > + help
> > +   Enables clockevent driver for Mediatek mt6765 timer.
> >
> >  config SPRD_TIMER
> >   bool "Spreadtrum timer driver" if EXPERT
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index c17ee32a7151..b1f06ce114f9 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -49,7 +49,8 @@ obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)+= samsung_pwm_timer.o
> >  obj-$(CONFIG_FSL_FTM_TIMER)  += timer-fsl-ftm.o
> >  obj-$(CONFIG_VF_PIT_TIMER)   += timer-vf-pit.o
> >  obj-$(CONFIG_CLKSRC_QCOM)+= timer-qcom.o
> > -obj-$(CONFIG_MTK_TIMER)  += timer-mediatek.o
> > +obj-$(CONFIG_TIMER_MEDIATEK_MT6577)  += timer-mediatek-mt6577.o
> > +obj-$(CONFIG_TIMER_MEDIATEK_MT6765)  += timer-mediatek-mt6765.o
> >  obj-$(CONFIG_CLKSRC_PISTACHIO)   += timer-pistachio.o
> >  obj-$(CONFIG_CLKSRC_TI_32K)  += timer-ti-32k.o
> >  obj-$(CONFIG_OXNAS_RPS_TIMER)+= timer-oxnas-rps.o
> > diff --git a/drivers/clocksource/timer-mediatek.c 
> > b/drivers/clocksource/timer-mediatek-mt6577.c
> > similarity index 69%
> > rename from drivers/clocksource/timer-mediatek.c
> > rename to drivers/clocksource/timer-mediatek-mt6577.c
> > index 9318edcd8963..9e5241d1876d 100644
> > --- a

Re: [PATCH] clocksource/drivers/timer-mediatek: optimize systimer irq clear flow on Mediatek Socs

2021-03-22 Thread Evan Benn 
On Thu, Mar 4, 2021 at 11:07 AM Fengquan Chen
 wrote:
>
> 1)ensure systimer is enabled before clear and disable interrupt, which only
> for systimer in Mediatek Socs.

Why does the timer need to be enabled before the interrupt can be
disabled? The datasheet I have does not suggest that this is required.

>
> 2)clear any pending timer-irq when shutdown to keep suspend flow clean,
> when use systimer as tick-broadcast timer
>
> Change-Id: Ia3eda83324af2fdaf5cbb3569a9bf020a11f8009
> Signed-off-by: Fengquan Chen 
> ---
>  drivers/clocksource/timer-mediatek.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/clocksource/timer-mediatek.c 
> b/drivers/clocksource/timer-mediatek.c
> index 9318edc..9f1f095dc 100644
> --- a/drivers/clocksource/timer-mediatek.c
> +++ b/drivers/clocksource/timer-mediatek.c
> @@ -75,6 +75,7 @@
>  static void mtk_syst_ack_irq(struct timer_of *to)

This function seems to be mis-named. It does more than just ack the irq.

>  {
> /* Clear and disable interrupt */
> +   writel(SYST_CON_EN, SYST_CON_REG(to));

This line seems to enable the timer and disable the interrupt.

> writel(SYST_CON_IRQ_CLR | SYST_CON_EN, SYST_CON_REG(to));

This line acks the interrupt and enables the timer and disables the interrupt.
Are these lines both necessary?
Maybe this function should just ack the interrupt without changing the
other bits.

>  }
>
> @@ -111,6 +112,9 @@ static int mtk_syst_clkevt_next_event(unsigned long ticks,
>
>  static int mtk_syst_clkevt_shutdown(struct clock_event_device *clkevt)
>  {
> +   /* Clear any irq */
> +   mtk_syst_ack_irq(to_timer_of(clkevt));
> +
> /* Disable timer */
> writel(0, SYST_CON_REG(to_timer_of(clkevt)));

This is a third write to the same register, I believe all 3 writes can
be combined into 1. Is that possible?

>
> --
> 1.8.1.1.dirty
>

Re: [PATCH] drm/amd/display: Set AMDGPU_DM_DEFAULT_MIN_BACKLIGHT to 0

2021-03-21 Thread Evan Benn 
On Sat, Mar 20, 2021 at 8:36 AM Alex Deucher  wrote:
>
> On Fri, Mar 19, 2021 at 5:31 PM Evan Benn  wrote:
> >
> > On Sat, 20 Mar 2021 at 02:10, Harry Wentland  wrote:
> > > On 2021-03-19 10:22 a.m., Alex Deucher wrote:
> > > > On Fri, Mar 19, 2021 at 3:23 AM Evan Benn  wrote:
> > > >>
> > > >> AMDGPU_DM_DEFAULT_MIN_BACKLIGHT was set to the value of 12
> > > >> to ensure no display backlight will flicker at low user brightness
> > > >> settings. However this value is quite bright, so for devices that do 
> > > >> not
> > > >> implement the ACPI ATIF
> > > >> ATIF_FUNCTION_QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS
> > > >> functionality the user cannot set the brightness to a low level even if
> > > >> the display would support such a low PWM.
> > > >>
> > > >> This ATIF feature is not implemented on for example AMD grunt 
> > > >> chromebooks.
> > > >>
> > > >> Signed-off-by: Evan Benn 
> > > >>
> > > >> ---
> > > >> I could not find a justification for the reason for the value. It has
> > > >> caused some noticable regression for users: 
> > > >> https://bugzilla.kernel.org/show_bug.cgi?id=203439>>>
> > > >> Maybe this can be either user controlled or userspace configured, but
> > > >> preventing users from turning their backlight dim seems wrong.
> > > >
> > > > My understanding is that some panels flicker if you set the min to a
> > > > value too low.  This was a safe minimum if the platform didn't specify
> > > > it's own safe minimum.  I think we'd just be trading one bug for
> > > > another (flickering vs not dim enough).  Maybe a whitelist or
> > > > blacklist would be a better solution?
> > > >
> > >
> > > Yeah, this is a NACK from me as-is for the reasons Alex described.
> >
> > Thanks Harry + Alex,
> >
> > I agree this solution is not the best.
> >
> > >
> > > I agree a whitelist approach might be best.
> >
> > Do you have any idea what an allowlist could be keyed on?
> > Is the flickering you observed here a function of the panel or the gpu
> > or some other component?
> > Maybe we could move the minimum level into the logic for that hardware.
> >
>
> Maybe the panel string from the EDID?  Either that or something from
> dmi data?  Harry would probably have a better idea.

One problem with keying from panel EDID is that for example the grunt chromebook
platform has more than 100 different panels already shipped. Add to that that
repair centers or people repairing their own device will use 'compatible'
panels. I'm sure the AMD windows laptops have even more variety!

>
> Alex
>
> > >
> > > Is this fix perhaps for OLED panels? If so we could use a different
> > > min-value for OLED panels that don't do PWM, but use 12 for everything 
> > > else.
> >
> > All the chromebooks I have worked with LCD + LED backlight have been
> > fine with a backlight set to 0.
> > We do have OLED panels too, but I'm not aware of what they do.
> >
> > > Harry
> > >
> > > > Alex
> > > >
> > > >
> > > >>
> > > >> Also reviewed here: 
> > > >> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2748377>>>
> > > >>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> > > >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > > >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > >> index 573cf17262da..0129bd69b94e 100644
> > > >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > >> @@ -3151,7 +3151,7 @@ static int amdgpu_dm_mode_config_init(struct 
> > > >> amdgpu_device *adev)
> > > >>  return 0;
> > > >>   }
> > > >>
> > > >> -#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 12
> > > >> +#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 0
> > > >>   #define AMDGPU_DM_DEFAULT_MAX_BACKLIGHT 255
> > > >>   #define AUX_BL_DEFAULT_TRANSITION_TIME_MS 50
> > > >>
> > > >> --
> > > >> 2.31.0.291.g576ba9dcdaf-goog
> > > >>
> > > >> ___
> > > >> dri-devel mailing list
> > > >> dri-de...@lists.freedesktop.org
> > > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel>> 
> > > >> ___
> > > > dri-devel mailing list
> > > > dri-de...@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel>>
> > >

Re: [PATCH] drm/amd/display: Set AMDGPU_DM_DEFAULT_MIN_BACKLIGHT to 0

2021-03-19 Thread Evan Benn 
On Sat, 20 Mar 2021 at 02:10, Harry Wentland  wrote:
> On 2021-03-19 10:22 a.m., Alex Deucher wrote:
> > On Fri, Mar 19, 2021 at 3:23 AM Evan Benn  wrote:
> >>
> >> AMDGPU_DM_DEFAULT_MIN_BACKLIGHT was set to the value of 12
> >> to ensure no display backlight will flicker at low user brightness
> >> settings. However this value is quite bright, so for devices that do not
> >> implement the ACPI ATIF
> >> ATIF_FUNCTION_QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS
> >> functionality the user cannot set the brightness to a low level even if
> >> the display would support such a low PWM.
> >>
> >> This ATIF feature is not implemented on for example AMD grunt chromebooks.
> >>
> >> Signed-off-by: Evan Benn 
> >>
> >> ---
> >> I could not find a justification for the reason for the value. It has
> >> caused some noticable regression for users: 
> >> https://bugzilla.kernel.org/show_bug.cgi?id=203439>>>
> >> Maybe this can be either user controlled or userspace configured, but
> >> preventing users from turning their backlight dim seems wrong.
> >
> > My understanding is that some panels flicker if you set the min to a
> > value too low.  This was a safe minimum if the platform didn't specify
> > it's own safe minimum.  I think we'd just be trading one bug for
> > another (flickering vs not dim enough).  Maybe a whitelist or
> > blacklist would be a better solution?
> >
>
> Yeah, this is a NACK from me as-is for the reasons Alex described.

Thanks Harry + Alex,

I agree this solution is not the best.

>
> I agree a whitelist approach might be best.

Do you have any idea what an allowlist could be keyed on?
Is the flickering you observed here a function of the panel or the gpu
or some other component?
Maybe we could move the minimum level into the logic for that hardware.

>
> Is this fix perhaps for OLED panels? If so we could use a different
> min-value for OLED panels that don't do PWM, but use 12 for everything else.

All the chromebooks I have worked with LCD + LED backlight have been
fine with a backlight set to 0.
We do have OLED panels too, but I'm not aware of what they do.

> Harry
>
> > Alex
> >
> >
> >>
> >> Also reviewed here: 
> >> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2748377>>>
> >>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> index 573cf17262da..0129bd69b94e 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -3151,7 +3151,7 @@ static int amdgpu_dm_mode_config_init(struct 
> >> amdgpu_device *adev)
> >>  return 0;
> >>   }
> >>
> >> -#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 12
> >> +#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 0
> >>   #define AMDGPU_DM_DEFAULT_MAX_BACKLIGHT 255
> >>   #define AUX_BL_DEFAULT_TRANSITION_TIME_MS 50
> >>
> >> --
> >> 2.31.0.291.g576ba9dcdaf-goog
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-de...@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel>> 
> >> ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel>>
>

[PATCH] drm/amd/display: Set AMDGPU_DM_DEFAULT_MIN_BACKLIGHT to 0

2021-03-18 Thread Evan Benn 
AMDGPU_DM_DEFAULT_MIN_BACKLIGHT was set to the value of 12
to ensure no display backlight will flicker at low user brightness
settings. However this value is quite bright, so for devices that do not
implement the ACPI ATIF
ATIF_FUNCTION_QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS
functionality the user cannot set the brightness to a low level even if
the display would support such a low PWM.

This ATIF feature is not implemented on for example AMD grunt chromebooks.

Signed-off-by: Evan Benn 

---
I could not find a justification for the reason for the value. It has
caused some noticable regression for users: 
https://bugzilla.kernel.org/show_bug.cgi?id=203439

Maybe this can be either user controlled or userspace configured, but
preventing users from turning their backlight dim seems wrong.

Also reviewed here: 
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2748377

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 573cf17262da..0129bd69b94e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3151,7 +3151,7 @@ static int amdgpu_dm_mode_config_init(struct 
amdgpu_device *adev)
return 0;
 }
 
-#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 12
+#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 0
 #define AMDGPU_DM_DEFAULT_MAX_BACKLIGHT 255
 #define AUX_BL_DEFAULT_TRANSITION_TIME_MS 50
 
-- 
2.31.0.291.g576ba9dcdaf-goog

[PATCH] scripts/coccinelle: Add script to detect sign extension

2021-03-18 Thread Evan Benn 
Hello,

I am attempting to create a coccinelle script that will detect possibly buggy
usage of the bitwise operators where integer promotion may result in bugs,
usually due to sign extension.

I know this script needs a lot more work, but I am just beginning to learn the
syntax of coccinelle. At this stage I am mainly looking for advice if this is
even worth continuing, or if I am on the wrong track entirely. 

Here is an example of the bug I hope to find:

https://lore.kernel.org/lkml/20210317013758.ga134...@roeck-us.net/

Where ints and unsigned are mixed in bitwise operations, and the sizes differ.

Thanks

Evan Benn

Signed-off-by: Evan Benn 
---

 .../coccinelle/tests/int_sign_extend.cocci| 35 +++
 1 file changed, 35 insertions(+)
 create mode 100644 scripts/coccinelle/tests/int_sign_extend.cocci

diff --git a/scripts/coccinelle/tests/int_sign_extend.cocci 
b/scripts/coccinelle/tests/int_sign_extend.cocci
new file mode 100644
index ..bad61e37e4e7
--- /dev/null
+++ b/scripts/coccinelle/tests/int_sign_extend.cocci
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Mixing signed and unsigned types in bitwise operations risks problems when
+/// the 'Usual arithmetic conversions' are applied.
+/// For example:
+/// https://lore.kernel.org/lkml/20210317013758.ga134...@roeck-us.net/
+/// When a signed int and an unsigned int are compared there is no problem.
+/// But if the unsigned is changed to a unsigned long, for example by using BIT
+/// the signed value will be sign-extended and could result in incorrect logic.
+// Confidence:
+// Copyright: (C) 2021 Evan Benn 
+// Comments:
+// Options:
+
+virtual context
+virtual org
+virtual report
+
+@r@
+position p;
+{int} s;
+{unsigned long} u;
+@@
+s@p & u
+
+@script:python depends on org@
+p << r.p;
+@@
+
+cocci.print_main("sign extension when comparing bits of signed and unsigned 
values", p)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0],"sign extension when comparing bits of 
signed and unsigned values")
-- 
2.31.0.291.g576ba9dcdaf-goog

Re: [PATCH 2/2] drivers/clocksource/mediatek: Ack and disable interrupts on shutdown

2021-03-18 Thread Evan Benn 
Hello,

There is a suspend failure on mt8173 chromebooks that use this timer.
The failure shows as an errno: -95 failure with none device.

I tracked this down to the arm trusted firmware aborting the suspend
due to this timer having a pending IRQ, due to not being disabled
during suspend / clockevents_shutdown.

Also reviewed here vs the 4.19 chromeos kernel branch:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2766504

Thanks

Evan Benn


On Thu, Mar 18, 2021 at 4:05 PM Evan Benn  wrote:
>
> set_state_shutdown is called during system suspend after interrupts have
> been disabled. If the timer has fired in the meantime, there will be
> a pending IRQ. So we ack that now and disable the timer. Without this
> ARM trusted firmware will abort the suspend due to the pending
> interrupt.
>
> Now always disable the IRQ in state transitions, and re-enable in
> set_periodic and next_event.
>
> Signed-off-by: Evan Benn 
> ---
>
>  drivers/clocksource/timer-mediatek-mt6577.c | 49 +
>  1 file changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/clocksource/timer-mediatek-mt6577.c 
> b/drivers/clocksource/timer-mediatek-mt6577.c
> index 9e5241d1876d..44598121585c 100644
> --- a/drivers/clocksource/timer-mediatek-mt6577.c
> +++ b/drivers/clocksource/timer-mediatek-mt6577.c
> @@ -54,13 +54,33 @@ static u64 notrace mtk_gpt_read_sched_clock(void)
> return readl_relaxed(gpt_sched_reg);
>  }
>
> +static void mtk_gpt_disable_ack_interrupts(struct timer_of *to, u8 timer)
> +{
> +   u32 val;
> +
> +   /* Disable interrupts */
> +   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> +   writel(val & ~GPT_IRQ_ENABLE(timer), timer_of_base(to) +
> +  GPT_IRQ_EN_REG);
> +
> +   /* Ack interrupts */
> +   writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
> +}
> +
>  static void mtk_gpt_clkevt_time_stop(struct timer_of *to, u8 timer)
>  {
> u32 val;
>
> +   /* Disable timer */
> val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
> writel(val & ~GPT_CTRL_ENABLE, timer_of_base(to) +
>GPT_CTRL_REG(timer));
> +
> +   /* This may be called with interrupts disabled,
> +* so we need to ack any interrupt that is pending
> +* Or for example ATF will prevent a suspend from completing.
> +*/
> +   mtk_gpt_disable_ack_interrupts(to, timer);
>  }
>
>  static void mtk_gpt_clkevt_time_setup(struct timer_of *to,
> @@ -74,8 +94,10 @@ static void mtk_gpt_clkevt_time_start(struct timer_of *to,
>  {
> u32 val;
>
> -   /* Acknowledge interrupt */
> -   writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
> +   /* Enable interrupts */
> +   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> +   writel(val | GPT_IRQ_ENABLE(timer),
> +  timer_of_base(to) + GPT_IRQ_EN_REG);
>
> val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
>
> @@ -148,21 +170,6 @@ __init mtk_gpt_setup(struct timer_of *to, u8 timer, u8 
> option)
>timer_of_base(to) + GPT_CTRL_REG(timer));
>  }
>
> -static void mtk_gpt_enable_irq(struct timer_of *to, u8 timer)
> -{
> -   u32 val;
> -
> -   /* Disable all interrupts */
> -   writel(0x0, timer_of_base(to) + GPT_IRQ_EN_REG);
> -
> -   /* Acknowledge all spurious pending interrupts */
> -   writel(0x3f, timer_of_base(to) + GPT_IRQ_ACK_REG);
> -
> -   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> -   writel(val | GPT_IRQ_ENABLE(timer),
> -  timer_of_base(to) + GPT_IRQ_EN_REG);
> -}
> -
>  static struct timer_of to = {
> .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
>
> @@ -193,6 +200,12 @@ static int __init mtk_gpt_init(struct device_node *node)
> if (ret)
> return ret;
>
> +   /* In case the firmware left the interrupts enabled
> +* disable and ack those now
> +*/
> +   mtk_gpt_disable_ack_interrupts(, TIMER_CLK_SRC);
> +   mtk_gpt_disable_ack_interrupts(, TIMER_CLK_EVT);
> +
> /* Configure clock source */
> mtk_gpt_setup(, TIMER_CLK_SRC, GPT_CTRL_OP_FREERUN);
> clocksource_mmio_init(timer_of_base() + GPT_CNT_REG(TIMER_CLK_SRC),
> @@ -206,8 +219,6 @@ static int __init mtk_gpt_init(struct device_node *node)
> clockevents_config_and_register(, timer_of_rate(),
> TIMER_SYNC_TICKS, 0x);
>
> -   mtk_gpt_enable_irq(, TIMER_CLK_EVT);
> -
> return 0;
>  }
>  TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
> --
> 2.31.0.rc2.261.g7f71774620-goog
>

[PATCH 2/2] drivers/clocksource/mediatek: Ack and disable interrupts on shutdown

2021-03-17 Thread Evan Benn 
set_state_shutdown is called during system suspend after interrupts have
been disabled. If the timer has fired in the meantime, there will be
a pending IRQ. So we ack that now and disable the timer. Without this
ARM trusted firmware will abort the suspend due to the pending
interrupt.

Now always disable the IRQ in state transitions, and re-enable in
set_periodic and next_event.

Signed-off-by: Evan Benn 
---

 drivers/clocksource/timer-mediatek-mt6577.c | 49 +
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/clocksource/timer-mediatek-mt6577.c 
b/drivers/clocksource/timer-mediatek-mt6577.c
index 9e5241d1876d..44598121585c 100644
--- a/drivers/clocksource/timer-mediatek-mt6577.c
+++ b/drivers/clocksource/timer-mediatek-mt6577.c
@@ -54,13 +54,33 @@ static u64 notrace mtk_gpt_read_sched_clock(void)
return readl_relaxed(gpt_sched_reg);
 }
 
+static void mtk_gpt_disable_ack_interrupts(struct timer_of *to, u8 timer)
+{
+   u32 val;
+
+   /* Disable interrupts */
+   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
+   writel(val & ~GPT_IRQ_ENABLE(timer), timer_of_base(to) +
+  GPT_IRQ_EN_REG);
+
+   /* Ack interrupts */
+   writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
+}
+
 static void mtk_gpt_clkevt_time_stop(struct timer_of *to, u8 timer)
 {
u32 val;
 
+   /* Disable timer */
val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
writel(val & ~GPT_CTRL_ENABLE, timer_of_base(to) +
   GPT_CTRL_REG(timer));
+
+   /* This may be called with interrupts disabled,
+* so we need to ack any interrupt that is pending
+* Or for example ATF will prevent a suspend from completing.
+*/
+   mtk_gpt_disable_ack_interrupts(to, timer);
 }
 
 static void mtk_gpt_clkevt_time_setup(struct timer_of *to,
@@ -74,8 +94,10 @@ static void mtk_gpt_clkevt_time_start(struct timer_of *to,
 {
u32 val;
 
-   /* Acknowledge interrupt */
-   writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
+   /* Enable interrupts */
+   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
+   writel(val | GPT_IRQ_ENABLE(timer),
+  timer_of_base(to) + GPT_IRQ_EN_REG);
 
val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
 
@@ -148,21 +170,6 @@ __init mtk_gpt_setup(struct timer_of *to, u8 timer, u8 
option)
   timer_of_base(to) + GPT_CTRL_REG(timer));
 }
 
-static void mtk_gpt_enable_irq(struct timer_of *to, u8 timer)
-{
-   u32 val;
-
-   /* Disable all interrupts */
-   writel(0x0, timer_of_base(to) + GPT_IRQ_EN_REG);
-
-   /* Acknowledge all spurious pending interrupts */
-   writel(0x3f, timer_of_base(to) + GPT_IRQ_ACK_REG);
-
-   val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
-   writel(val | GPT_IRQ_ENABLE(timer),
-  timer_of_base(to) + GPT_IRQ_EN_REG);
-}
-
 static struct timer_of to = {
.flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
 
@@ -193,6 +200,12 @@ static int __init mtk_gpt_init(struct device_node *node)
if (ret)
return ret;
 
+   /* In case the firmware left the interrupts enabled
+* disable and ack those now
+*/
+   mtk_gpt_disable_ack_interrupts(, TIMER_CLK_SRC);
+   mtk_gpt_disable_ack_interrupts(, TIMER_CLK_EVT);
+
/* Configure clock source */
mtk_gpt_setup(, TIMER_CLK_SRC, GPT_CTRL_OP_FREERUN);
clocksource_mmio_init(timer_of_base() + GPT_CNT_REG(TIMER_CLK_SRC),
@@ -206,8 +219,6 @@ static int __init mtk_gpt_init(struct device_node *node)
clockevents_config_and_register(, timer_of_rate(),
TIMER_SYNC_TICKS, 0x);
 
-   mtk_gpt_enable_irq(, TIMER_CLK_EVT);
-
return 0;
 }
 TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH 1/2] drivers/clocksource/mediatek: Split mediatek drivers into 2 files

2021-03-17 Thread Evan Benn 
mtk_gpt and mtk_syst drivers for mt6577 and mt6765 devices were not
sharing any code. So split them into separate files.

Signed-off-by: Evan Benn 
---

 arch/arm/mach-mediatek/Kconfig|   3 +-
 arch/arm64/Kconfig.platforms  |   3 +-
 drivers/clocksource/Kconfig   |  13 +-
 drivers/clocksource/Makefile  |   3 +-
 ...mer-mediatek.c => timer-mediatek-mt6577.c} | 100 -
 drivers/clocksource/timer-mediatek-mt6765.c   | 135 ++
 6 files changed, 151 insertions(+), 106 deletions(-)
 rename drivers/clocksource/{timer-mediatek.c => timer-mediatek-mt6577.c} (69%)
 create mode 100644 drivers/clocksource/timer-mediatek-mt6765.c

diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
index 9e0f592d87d8..8686f992c4b6 100644
--- a/arch/arm/mach-mediatek/Kconfig
+++ b/arch/arm/mach-mediatek/Kconfig
@@ -4,7 +4,8 @@ menuconfig ARCH_MEDIATEK
depends on ARCH_MULTI_V7
select ARM_GIC
select PINCTRL
-   select MTK_TIMER
+   select TIMER_MEDIATEK_MT6577
+   select TIMER_MEDIATEK_MT6765
select MFD_SYSCON
help
  Support for Mediatek MT65xx & MT81xx SoCs
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index cdfd5fed457f..d4752375ab0b 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -161,7 +161,8 @@ config ARCH_MEDIATEK
bool "MediaTek SoC Family"
select ARM_GIC
select PINCTRL
-   select MTK_TIMER
+   select TIMER_MEDIATEK_MT6577
+   select TIMER_MEDIATEK_MT6765
help
  This enables support for MediaTek MT27xx, MT65xx, MT76xx
  & MT81xx ARMv8 SoCs
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 39aa21d01e05..d697c799284e 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -438,13 +438,20 @@ config OXNAS_RPS_TIMER
 config SYS_SUPPORTS_SH_CMT
bool
 
-config MTK_TIMER
-   bool "Mediatek timer driver" if COMPILE_TEST
+config TIMER_MEDIATEK_MT6577
+   bool "Mediatek mt6577 timer driver" if COMPILE_TEST
depends on HAS_IOMEM
select TIMER_OF
select CLKSRC_MMIO
help
- Support for Mediatek timer driver.
+ Enables clocksource and clockevent driver for Mediatek mt6577 timer.
+
+config TIMER_MEDIATEK_MT6765
+   bool "Mediatek mt6765 timer driver" if COMPILE_TEST
+   depends on HAS_IOMEM
+   select TIMER_OF
+   help
+ Enables clockevent driver for Mediatek mt6765 timer.
 
 config SPRD_TIMER
bool "Spreadtrum timer driver" if EXPERT
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c17ee32a7151..b1f06ce114f9 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -49,7 +49,8 @@ obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)  += samsung_pwm_timer.o
 obj-$(CONFIG_FSL_FTM_TIMER)+= timer-fsl-ftm.o
 obj-$(CONFIG_VF_PIT_TIMER) += timer-vf-pit.o
 obj-$(CONFIG_CLKSRC_QCOM)  += timer-qcom.o
-obj-$(CONFIG_MTK_TIMER)+= timer-mediatek.o
+obj-$(CONFIG_TIMER_MEDIATEK_MT6577)+= timer-mediatek-mt6577.o
+obj-$(CONFIG_TIMER_MEDIATEK_MT6765)+= timer-mediatek-mt6765.o
 obj-$(CONFIG_CLKSRC_PISTACHIO) += timer-pistachio.o
 obj-$(CONFIG_CLKSRC_TI_32K)+= timer-ti-32k.o
 obj-$(CONFIG_OXNAS_RPS_TIMER)  += timer-oxnas-rps.o
diff --git a/drivers/clocksource/timer-mediatek.c 
b/drivers/clocksource/timer-mediatek-mt6577.c
similarity index 69%
rename from drivers/clocksource/timer-mediatek.c
rename to drivers/clocksource/timer-mediatek-mt6577.c
index 9318edcd8963..9e5241d1876d 100644
--- a/drivers/clocksource/timer-mediatek.c
+++ b/drivers/clocksource/timer-mediatek-mt6577.c
@@ -47,86 +47,8 @@
 #define GPT_CNT_REG(val)(0x08 + (0x10 * (val)))
 #define GPT_CMP_REG(val)(0x0C + (0x10 * (val)))
 
-/* system timer */
-#define SYST_BASE   (0x40)
-
-#define SYST_CON(SYST_BASE + 0x0)
-#define SYST_VAL(SYST_BASE + 0x4)
-
-#define SYST_CON_REG(to)(timer_of_base(to) + SYST_CON)
-#define SYST_VAL_REG(to)(timer_of_base(to) + SYST_VAL)
-
-/*
- * SYST_CON_EN: Clock enable. Shall be set to
- *   - Start timer countdown.
- *   - Allow timeout ticks being updated.
- *   - Allow changing interrupt functions.
- *
- * SYST_CON_IRQ_EN: Set to allow interrupt.
- *
- * SYST_CON_IRQ_CLR: Set to clear interrupt.
- */
-#define SYST_CON_EN  BIT(0)
-#define SYST_CON_IRQ_EN  BIT(1)
-#define SYST_CON_IRQ_CLR BIT(4)
-
 static void __iomem *gpt_sched_reg __read_mostly;
 
-static void mtk_syst_ack_irq(struct timer_of *to)
-{
-   /* Clear and disable interrupt */
-   writel(SYST_CON_IRQ_CLR | SYST_CON_EN, SYST_CON_REG(to));
-}
-
-static irqreturn_t mtk_syst_handler(int irq, void *dev_id)
-{
-   str

[PATCH 2/2] platform/chrome: cros_ec_proto: Add LID and BATTERY to default mask

2020-12-09 Thread Evan Benn 
After 'platform/chrome: cros_ec_proto: Use EC_HOST_EVENT_MASK not BIT'
some of the flags are not quite correct.
LID_CLOSED is used to suspend the device, so it makes sense to ignore that.
BATTERY events are also frequent and causing spurious wakes on elm/hana
mt8173 devices.

Fixes: c214e564acb2ad9463293ab9c109bfdae91fbeaf
Signed-off-by: Evan Benn 
---

 drivers/platform/chrome/cros_ec_proto.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c 
b/drivers/platform/chrome/cros_ec_proto.c
index 4e442175612d..ea5149efcbea 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -526,9 +526,11 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
 * power), not wake up.
 */
ec_dev->host_event_wake_mask = U32_MAX &
-   ~(EC_HOST_EVENT_MASK(EC_HOST_EVENT_AC_DISCONNECTED) |
+   ~(EC_HOST_EVENT_MASK(EC_HOST_EVENT_LID_CLOSED) |
+ EC_HOST_EVENT_MASK(EC_HOST_EVENT_AC_DISCONNECTED) |
  EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_LOW) |
  EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_CRITICAL) |
+ EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY) |
  EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU) |
  EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_STATUS));
/*
-- 
2.20.1

[PATCH 1/2] platform/chrome: cros_ec_proto: Use EC_HOST_EVENT_MASK not BIT

2020-12-09 Thread Evan Benn 
The host_event_code enum is 1-based, use EC_HOST_EVENT_MASK not BIT to
generate the intended mask. This patch changes the behaviour of the
mask, a following patch will restore the intended behaviour:
'Add LID and BATTERY to default mask'

Fixes: c214e564acb2ad9463293ab9c109bfdae91fbeaf
Signed-off-by: Evan Benn 
---

 drivers/platform/chrome/cros_ec_proto.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c 
b/drivers/platform/chrome/cros_ec_proto.c
index 0ecee8b8773d..4e442175612d 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -526,11 +526,11 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
 * power), not wake up.
 */
ec_dev->host_event_wake_mask = U32_MAX &
-   ~(BIT(EC_HOST_EVENT_AC_DISCONNECTED) |
- BIT(EC_HOST_EVENT_BATTERY_LOW) |
- BIT(EC_HOST_EVENT_BATTERY_CRITICAL) |
- BIT(EC_HOST_EVENT_PD_MCU) |
- BIT(EC_HOST_EVENT_BATTERY_STATUS));
+   ~(EC_HOST_EVENT_MASK(EC_HOST_EVENT_AC_DISCONNECTED) |
+ EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_LOW) |
+ EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_CRITICAL) |
+ EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU) |
+ EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_STATUS));
/*
 * Old ECs may not support this command. Complain about all
 * other errors.
-- 
2.20.1

[PATCH] media: mtk-vcodec: Fix order of log arguments

2020-11-25 Thread Evan Benn 
Signed-off-by: Evan Benn 

---

 drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c 
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c
index a3c7a380c9308..785ec0df445ec 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c
@@ -27,11 +27,11 @@ int mtk_vcodec_wait_for_done_ctx(struct mtk_vcodec_ctx  
*ctx, int command,
 
if (!ret) {
status = -1;/* timeout */
-   mtk_v4l2_err("[%d] cmd=%d, ctx->type=%d, 
wait_event_interruptible_timeout time=%ums out %d %d!",
+   mtk_v4l2_err("[%d] ctx->type=%d, cmd=%d, 
wait_event_interruptible_timeout time=%ums out %d %d!",
ctx->id, ctx->type, command, timeout_ms,
ctx->int_cond, ctx->int_type);
} else if (-ERESTARTSYS == ret) {
-   mtk_v4l2_err("[%d] cmd=%d, ctx->type=%d, 
wait_event_interruptible_timeout interrupted by a signal %d %d",
+   mtk_v4l2_err("[%d] ctx->type=%d, cmd=%d, 
wait_event_interruptible_timeout interrupted by a signal %d %d",
ctx->id, ctx->type, command, ctx->int_cond,
ctx->int_type);
status = -1;
-- 
2.29.2.454.gaff20da3a2-goog

Re: [PATCH] MAINTAINERS: rectify entry in ARM SMC WATCHDOG DRIVER

2020-06-04 Thread Evan Benn 
AFAICT this has now been merged upstream, I'm not sure what action to take:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5c24a28b4eb842ad1256496be6ae01bab15f1dcb
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72a9e7fea5866fc471fda78f05f166595c8c6ba6

On Wed, Jun 3, 2020 at 9:22 AM Evan Benn  wrote:
>
> Apologies for that slip up.
>
> Reviewed-by: Evan Benn 
>
> On Wed, Jun 3, 2020 at 6:16 AM Julius Werner  wrote:
> >
> > Reviewed-by: Julius Werner

Re: [PATCH] MAINTAINERS: rectify entry in ARM SMC WATCHDOG DRIVER

2020-06-02 Thread Evan Benn 
Apologies for that slip up.

Reviewed-by: Evan Benn 

On Wed, Jun 3, 2020 at 6:16 AM Julius Werner  wrote:
>
> Reviewed-by: Julius Werner

[PATCH v6 0/2] Add a watchdog driver that uses ARM Secure Monitor Calls.

2020-05-04 Thread Evan Benn 
This is currently supported in firmware deployed on oak, hana and elm mt8173
chromebook devices. The kernel driver is written to be a generic SMC
watchdog driver.

Arm Trusted Firmware upstreaming review:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3405

Patch to add oak, hana, elm device tree:

https://lore.kernel.org/linux-arm-kernel/20200110073730.213789-1-hsi...@chromium.org/
I would like to add the device tree support after the above patch is
accepted.

Changes in v6:
- Don't use dt default
- Use default arm,smc-id value if non provided by dt

Changes in v5:
- Change compatible to arm,smc-wdt
- Make timeleft return 0 on error
- Use type punning on smc_func_id to save an alloc
- Change compatible to arm,smc-wdt

Changes in v4:
- Add arm,smc-id property
- Get smc-id from of property
- Return a1 instead of a0 in timeleft

Changes in v3:
- Change name back to arm
- Add optional get_timeleft op
- change name to arm_smc_wdt

Changes in v2:
- Change name arm > mt8173
- use watchdog_stop_on_reboot
- use watchdog_stop_on_unregister
- use devm_watchdog_register_device
- remove smcwd_shutdown, smcwd_remove
- change error codes

Evan Benn (1):
  dt-bindings: watchdog: Add ARM smc wdt for mt8173 watchdog

Julius Werner (1):
  watchdog: Add new arm_smc_wdt watchdog driver

 .../bindings/watchdog/arm-smc-wdt.yaml|  37 
 MAINTAINERS   |   7 +
 arch/arm64/configs/defconfig  |   1 +
 drivers/watchdog/Kconfig  |  13 ++
 drivers/watchdog/Makefile |   1 +
 drivers/watchdog/arm_smc_wdt.c| 188 ++
 6 files changed, 247 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
 create mode 100644 drivers/watchdog/arm_smc_wdt.c

-- 
2.26.2.526.g744177e7f7-goog

[PATCH v6 2/2] watchdog: Add new arm_smc_wdt watchdog driver

2020-05-04 Thread Evan Benn 
From: Julius Werner 

This patch adds a watchdog driver that can be used on ARM systems
with the appropriate watchdog implemented in Secure Monitor firmware.
The driver communicates with firmware via a Secure Monitor Call.
This may be useful for platforms using TrustZone that want
the Secure Monitor firmware to have the final control over the watchdog.

This is implemented on mt8173 chromebook devices oak, elm and hana in
arm trusted firmware file plat/mediatek/mt8173/drivers/wdt/wdt.c.

Signed-off-by: Julius Werner 
Signed-off-by: Evan Benn 

---

Changes in v6:
- Use default arm,smc-id value if non provided by dt

Changes in v5:
- Make timeleft return 0 on error
- Use type punning on smc_func_id to save an alloc
- Change compatible to arm,smc-wdt

Changes in v4:
- Get smc-id from of property
- Return a1 instead of a0 in timeleft

Changes in v3:
- Add optional get_timeleft op
- change name to arm_smc_wdt

Changes in v2:
- use watchdog_stop_on_reboot
- use watchdog_stop_on_unregister
- use devm_watchdog_register_device
- remove smcwd_shutdown, smcwd_remove
- change error codes

 MAINTAINERS|   1 +
 arch/arm64/configs/defconfig   |   1 +
 drivers/watchdog/Kconfig   |  13 +++
 drivers/watchdog/Makefile  |   1 +
 drivers/watchdog/arm_smc_wdt.c | 188 +
 5 files changed, 204 insertions(+)
 create mode 100644 drivers/watchdog/arm_smc_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0f2b39767bfa9..2b782bbff200a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1462,6 +1462,7 @@ M:Julius Werner 
 R: Evan Benn 
 S: Maintained
 F: devicetree/bindings/watchdog/arm-smc-wdt.yaml
+F: drivers/watchdog/arm_smc_wdt.c
 
 ARM SMMU DRIVERS
 M: Will Deacon 
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 24e534d850454..0619df80f7575 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -513,6 +513,7 @@ CONFIG_UNIPHIER_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_ARM_SP805_WATCHDOG=y
 CONFIG_ARM_SBSA_WATCHDOG=y
+CONFIG_ARM_SMC_WATCHDOG=y
 CONFIG_S3C2410_WATCHDOG=y
 CONFIG_DW_WATCHDOG=y
 CONFIG_SUNXI_WATCHDOG=m
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0663c604bd642..c440b576d23bf 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -867,6 +867,19 @@ config DIGICOLOR_WATCHDOG
  To compile this driver as a module, choose M here: the
  module will be called digicolor_wdt.
 
+config ARM_SMC_WATCHDOG
+   tristate "ARM Secure Monitor Call based watchdog support"
+   depends on ARM || ARM64
+   depends on OF
+   depends on HAVE_ARM_SMCCC
+   select WATCHDOG_CORE
+   help
+ Say Y here to include support for a watchdog timer
+ implemented by the EL3 Secure Monitor on ARM platforms.
+ Requires firmware support.
+ To compile this driver as a module, choose M here: the
+ module will be called arm_smc_wdt.
+
 config LPC18XX_WATCHDOG
tristate "LPC18xx/43xx Watchdog"
depends on ARCH_LPC18XX || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 6de2e4ceef190..97bed1d3d97cb 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
 obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o
 obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
 obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o
+obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o
 
 # X86 (i386 + ia64 + x86_64) Architecture
 obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
diff --git a/drivers/watchdog/arm_smc_wdt.c b/drivers/watchdog/arm_smc_wdt.c
new file mode 100644
index 0..8f3d0c3a005fb
--- /dev/null
+++ b/drivers/watchdog/arm_smc_wdt.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ARM Secure Monitor Call watchdog driver
+ *
+ * Copyright 2020 Google LLC.
+ * Julius Werner 
+ * Based on mtk_wdt.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRV_NAME   "arm_smc_wdt"
+#define DRV_VERSION"1.0"
+
+enum smcwd_call {
+   SMCWD_INIT  = 0,
+   SMCWD_SET_TIMEOUT   = 1,
+   SMCWD_ENABLE= 2,
+   SMCWD_PET   = 3,
+   SMCWD_GET_TIMELEFT  = 4,
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+static unsigned int timeout;
+
+static int smcwd_call(struct watchdog_device *wdd, enum smcwd_call call,
+ unsigned long arg, struct arm_smccc_res *res)
+{
+   struct arm_smccc_res local_res;
+
+   if (!res)
+   res = _res;
+
+   arm_smccc_smc((u32)(uintptr_t)watchdog_get_drvdata(wdd), call, arg, 0,
+ 0, 0, 0, 0, res);
+
+   if (res->a0 == PSCI_RET_NOT_SUPPORTED)
+   return -ENODEV;
+   if (res->a0 == PSCI_RET_INVALID_PARAMS)
+

[PATCH v6 1/2] dt-bindings: watchdog: Add ARM smc wdt for mt8173 watchdog

2020-05-04 Thread Evan Benn 
This watchdog can be used on ARM systems with a Secure
Monitor firmware to forward watchdog operations to
firmware via a Secure Monitor Call.

Signed-off-by: Evan Benn 

---

Changes in v6:
- Don't use dt default

Changes in v5:
- Change compatible to arm,smc-wdt

Changes in v4:
- Add arm,smc-id property

Changes in v3:
- Change name back to arm

Changes in v2:
- Change name arm > mt8173

 .../bindings/watchdog/arm-smc-wdt.yaml| 37 +++
 MAINTAINERS   |  6 +++
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml 
b/Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
new file mode 100644
index 0..bec651541e0c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/arm-smc-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Secure Monitor Call based watchdog
+
+allOf:
+  - $ref: "watchdog.yaml#"
+
+maintainers:
+  - Julius Werner 
+
+properties:
+  compatible:
+enum:
+  - arm,smc-wdt
+  arm,smc-id:
+allOf:
+  - $ref: /schemas/types.yaml#/definitions/uint32
+description: |
+  The ATF smc function id used by the firmware.
+  Defaults to 0x82003D06 if unset.
+
+required:
+  - compatible
+
+examples:
+  - |
+watchdog {
+  compatible = "arm,smc-wdt";
+  arm,smc-id = <0x82003D06>;
+  timeout-sec = <15>;
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index b816a453b10eb..0f2b39767bfa9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1457,6 +1457,12 @@ S:   Maintained
 F: Documentation/devicetree/bindings/interrupt-controller/arm,vic.txt
 F: drivers/irqchip/irq-vic.c
 
+ARM SMC WATCHDOG DRIVER
+M:     Julius Werner 
+R: Evan Benn 
+S: Maintained
+F: devicetree/bindings/watchdog/arm-smc-wdt.yaml
+
 ARM SMMU DRIVERS
 M: Will Deacon 
 R: Robin Murphy 
-- 
2.26.2.526.g744177e7f7-goog