Re: [linux-sunxi] Re: [PATCH 9/9] arm64: dts: allwinner: Move wakeup-capable IRQs to r_intc

2020-01-13 Thread Samuel Holland
Hello,

On 1/13/20 11:30 AM, pune wrote:
> I am using Banana bi bsp for android which in turn using lichee kernel 3.10

Unfortunately, this is the reason you are not getting much help. The BSP kernel
works very differently from mainline Linux, especially with regard to power
management. Allwinner provides a proprietary firmware blob that communicates
using a custom protocol, and they added a lot of code to their Linux fork to
make it play nice with that firmware.

The patchset you replied to is for mainline Linux, where the firmware design,
communication protocol, memory layout, hardware ownership rules, etc., are all
very different.

> where i am not able bring up device from deep sleep state
> by pressing power button, i can see the power button irq is configured for GIC
> parent. Could you please let me know how to make this
> irq en-route to r_intc so that system can resume on power button press.

The IRQ from the AXP PMIC is the NMI--it always goes through R_INTC in addition
to the GIC. In the BSP kernel, the firmware takes care of configuring it, so
Linux doesn't know about the topology.

As for your suspend issue, here are some guesses based on what I remember from
the BSP code:

1) Make sure that an ARISC firmware is actually getting loaded. You cannot have
deep sleep without it. There should be some messages from Boot0 or U-Boot
telling you that it loaded scp.bin and started the ARISC.

2) Check the ARISC's UART output to see if it does anything when you put the
board to sleep, or when you try to wake it up. It's at GPIO pins PL2/PL3, which
are test points TP1/TP2 on the Banana Pi M64.

3) Check the ARISC parameters in the FEX or the device tree. That specifies
which wakeup sources are enabled in the ARISC firmware (make sure you have
wakeup from the PMIC enabled). It also controls properties like DRAM
self-refresh (try turning that off and see if it helps). Try playing with the
other parameters. Some of them may need to match your ATF or U-Boot, so maybe
try a different scp.bin if you have one.

4) As a last resort, there is a register in the RTC power domain at 0x01f0010c,
where the firmware writes a status code at several places during the
suspend/resume process. If you can reset the board without removing power from
the RTC, you can use that to troubleshoot. First clear the register, then
attempt to suspend/resume; when it fails, reset the board and read the register
again. You'd need to look at a disassembly of the ARISC firmware to interpret
the value; see here: https://github.com/smaeul/sunxi-blobs

I'm afraid that's all the help I'll be able to give, since I never use the BSP
kernel.

Good luck,
Samuel

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/f01ae15e-d90a-6f73-4dcb-4804e1491549%40sholland.org.


[linux-sunxi] Re: [PATCH v3 0/8] X-Powers Power Supply Improvements

2020-01-13 Thread Sebastian Reichel
Hi,

I queued patches 1-4, patch 5 does not apply and seems to be based
on an older tree.

-- Sebastian

On Sun, Jan 12, 2020 at 09:53:02PM -0600, Samuel Holland wrote:
> This series adds some improvements to the axp20x* power supply drivers
> to better support suspend/resume and use on mobile devices.
> 
> The first two patches fix bugs I found while testing the ONLINE control
> added in later patches.
> 
> Patches 3 and 7 allow userspace to take the power supplies offline.
> Patches 4 and 8 allow userspace to control the wakeup behavior.
> 
> Patch 9 avoids polling USB VBUS presence when possible. While working on
> the RSB driver, I was seeing ~50 transfers per second, while idle and
> tracked it down to this VBUS polling (20 reads/second). The polling
> often caused the CPU to clock up and back down, which triggered the
> remaining transfers (changes to the CPU voltage).
> 
> Unfortunately, I don't see a way to avoid the polling when running on
> battery (where it matters most), other than to move the polling back to
> the USB PHY driver.
> 
> Changes since v2:
>  - Patch 1 was merged
>  - Only check ACIN_PATH_SEL when necessary (1)
>  - Update commit message (5)
>  - Avoided reordering lines until/unless necessary (5, 7)
>  - Update comment and add ID check in axp20x_usb_power_set_property
>(it seemed more correct than adding another comment) (6)
>  - Add Reviewed-by where there were no comments (2-4, 7-8)
> 
> Changes since v1:
>  - Add patches 1-2
>  - Shift value properly in calls to regmap_update_bits (3, 7)
>  - Use #ifdef instead of #if to avoid -Wundef warnings (4, 8)
>  - Poll once after an IRQ, instead of setting power->online in the IRQ (9)
>  - Poll once on resume, in case the state changed during suspend (9)
> 
> Samuel Holland (8):
>   power: supply: axp20x_ac_power: Fix reporting online status
>   power: supply: axp20x_ac_power: Allow offlining
>   power: supply: axp20x_ac_power: Add wakeup control
>   power: supply: axp20x_usb_power: Remove unused device_node
>   power: supply: axp20x_usb_power: Use a match structure
>   power: supply: axp20x_usb_power: Allow offlining
>   power: supply: axp20x_usb_power: Add wakeup control
>   power: supply: axp20x_usb_power: Only poll while offline
> 
>  drivers/power/supply/axp20x_ac_power.c  | 131 +++---
>  drivers/power/supply/axp20x_usb_power.c | 219 ++--
>  2 files changed, 276 insertions(+), 74 deletions(-)
> 
> -- 
> 2.23.0
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200114001729.dgozjk4eaxjp7fzz%40earth.universe.


signature.asc
Description: PGP signature


[linux-sunxi] Re: [PATCH v6 1/6] dt-bindings: mailbox: Add a sun6i message box binding

2020-01-13 Thread Rob Herring
On Sun, 12 Jan 2020 23:18:47 -0600, Samuel Holland wrote:
> This mailbox hardware is present in Allwinner sun6i, sun8i, sun9i, and
> sun50i SoCs. Add a device tree binding for it.
> 
> Signed-off-by: Samuel Holland 
> ---
>  .../mailbox/allwinner,sun6i-a31-msgbox.yaml   | 80 +++
>  1 file changed, 80 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/mailbox/allwinner,sun6i-a31-msgbox.yaml
> 

Reviewed-by: Rob Herring 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200113223851.GA30654%40bogus.


[linux-sunxi] Re: [PATCH 2/3] Input: axp20x-pek - Respect userspace wakeup configuration

2020-01-13 Thread Dmitry Torokhov
On Mon, Jan 13, 2020 at 11:48:35AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 13-01-2020 04:20, Samuel Holland wrote:
> > Unlike most other power button drivers, this driver unconditionally
> > enables its wakeup IRQ. It should be using device_may_wakeup() to
> > respect the userspace configuration of wakeup sources.
> > 
> > Because the AXP20x MFD device uses regmap-irq, the AXP20x PEK IRQs are
> > nested off of regmap-irq's threaded interrupt handler. The device core
> > ignores such interrupts, so to actually disable wakeup, we must
> > explicitly disable all non-wakeup interrupts during suspend.
> > 
> > Signed-off-by: Samuel Holland 
> > ---
> >   drivers/input/misc/axp20x-pek.c | 42 -
> >   1 file changed, 41 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/misc/axp20x-pek.c 
> > b/drivers/input/misc/axp20x-pek.c
> > index 7d0ee5bececb..38cd4a4aeb65 100644
> > --- a/drivers/input/misc/axp20x-pek.c
> > +++ b/drivers/input/misc/axp20x-pek.c
> > @@ -280,7 +280,7 @@ static int axp20x_pek_probe_input_device(struct 
> > axp20x_pek *axp20x_pek,
> > }
> > if (axp20x_pek->axp20x->variant == AXP288_ID)
> > -   enable_irq_wake(axp20x_pek->irq_dbr);
> > +   device_init_wakeup(&pdev->dev, true);
> > return 0;
> >   }
> > @@ -352,6 +352,45 @@ static int axp20x_pek_probe(struct platform_device 
> > *pdev)
> > return 0;
> >   }
> > +#if CONFIG_PM_SLEEP
> 
> As the kbuild test robot pointed out, you need to use #ifdef here.

I prefer __maybe_unused as this gives more compile coverage.

Thanks.

-- 
Dmitry

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200113213855.GB47797%40dtor-ws.


[linux-sunxi] Re: [PATCH 3/3] Input: axp20x-pek - Enable wakeup for all AXP variants

2020-01-13 Thread Dmitry Torokhov
Hi Samuel,

On Sun, Jan 12, 2020 at 09:20:32PM -0600, Samuel Holland wrote:
> There are many devices, including several mobile battery-powered
> devices, using other AXP variants as their PMIC. Enable them to use
> the power key as a wakeup source.

Are these X86 or ARM devices? If anything, I'd prefer individual drivers
not declare themselves as wakeup sources unconditionally. With devic
etree we have standard "wakeup-source" property, but I am not quite sure
what's the latest on X86...

Thanks.

-- 
Dmitry

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200113212654.GA47797%40dtor-ws.


[linux-sunxi] [PATCH] arm64: dts: allwinner: h6: tanix-tx6: Use internal oscillator

2020-01-13 Thread Jernej Skrabec
Tanix TX6 doesn't have external 32 kHz oscillator, so switch RTC clock
to internal one.

Signed-off-by: Jernej Skrabec 
---

While this patch gives one possible solution, I mainly want to start
discussion why Allwinner SoC dtsi reference external 32 kHz crystal
although some boards don't have it. My proposal would be to make clock
property optional, based on the fact if external crystal is present or
not. However, I'm not sure if that is possible at this point or not.
Driver also considers missing clock property as deprecated (old DT) [1],
so this might complicate things even further.

What do you think?

Best regards,
Jernej

[1] https://elixir.bootlin.com/linux/latest/source/drivers/rtc/rtc-sun6i.c#L263

 arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
index 83e6cb0e59ce..af3aebda47bb 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
@@ -91,6 +91,12 @@ &r_ir {
status = "okay";
 };
 
+/* This board doesn't have external 32 kHz crystal. */
+&rtc {
+   assigned-clocks = <&rtc 0>;
+   assigned-clock-parents = <&rtc 2>;
+};
+
 &uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_ph_pins>;
-- 
2.24.1

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20200113180720.77461-1-jernej.skrabec%40siol.net.


[linux-sunxi] Re: [PATCH 9/9] arm64: dts: allwinner: Move wakeup-capable IRQs to r_intc

2020-01-13 Thread pune
Hi Samuel

I am using Banana bi bsp for android which in turn using lichee kernel 3.10 
where i am not able bring up device from deep sleep state
by pressing power button, i can see the power button irq is configured for 
GIC parent. Could you please let me know how to make this 
irq en-route to r_intc so that system can resume on power button press.

Please find for some more details below.

cat /proc/interrupts | grep axp
 64: 15  0  0  0   GIC  axp

axp-mfd.c -> where irq registered for device

#ifdef CONFIG_SUNXI_ARISC
ret = request_irq(dev->irq_number, axp_mfd_irq_cb,
IRQF_SHARED|IRQF_DISABLED|IRQF_NO_SUSPEND, "axp", dev);
if (ret) {
printk(KERN_ERR "failed to request irq %d\n",
dev->irq_number);
goto out_free_dev;
}
#endif


dt entry: PFA

your help will be really appreciable, the patches you shared may not 
applicable for lichee kernel.


-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/0c81a5a9-8b9b-4c15-95c9-5cf962609e18%40googlegroups.com.


sunxi-banana-image-a64-dev-working-hdmi.dts
Description: Binary data


[linux-sunxi] Re: [PATCH 1/3] Input: axp20x-pek - Remove unique wakeup event handling

2020-01-13 Thread Hans de Goede

On 13-01-2020 11:41, Hans de Goede wrote:

Hi,

On 13-01-2020 04:20, Samuel Holland wrote:

This driver attempts to avoid reporting wakeup events to userspace by
clearing a possible pending IRQ before IRQs are enabled during resume.
The assumption seems to be that userspace cannot cope with a KEY_POWER
press during resume. However, no other input driver does this, so it
would be a bug that such events are missing with this driver.

Furthermore, for PMICs connected via I2C or RSB, it is not possible to
update the regmap during the noirq resume phase, because the bus
controller drivers require IRQs to perform bus transactions. And the
resume hook cannot move to a later phase, because then it would race
with the power key IRQ handler.

So the best solution seems to be simply removing the hook.


Hmm, I'm not sure this is a good idea, let me give you some background
info on this:

This hook was handled because on X86 systems/laptops when waking
them up typically the power-button does not send a KEY_POWER press event
when the system was woken up through the power-button.

So normal (e.g. Debian, Fedora) userspace does not expect this event
and will directly go to sleep again because that is the default behavior
on a KEY_POWER event.


p.s.

The main reason why typical userspace does not expect the KEY_POWER
event is because most of the devices which run mainline and have
suspend/resume working and are using the ACPI button driver for
the power-button: drivers/acpi/button.c, which has:

acpi_pm_wakeup_event(&device->dev);
if (button->suspended)
break;

keycode = test_bit(KEY_SLEEP, input->keybit) ?
KEY_SLEEP : KEY_POWER;
input_report_key(input, keycode, 1);
input_sync(input);
input_report_key(input, keycode, 0);
input_sync(input);

And:

static int acpi_button_suspend(struct device *dev)
{
struct acpi_device *device = to_acpi_device(dev);
struct acpi_button *button = acpi_driver_data(device);

button->suspended = true;
return 0;
}

static int acpi_button_resume(struct device *dev)
{
struct acpi_device *device = to_acpi_device(dev);
struct acpi_button *button = acpi_driver_data(device);

button->suspended = false;
return 0;
}

So when the ACPI notify for the button runs on resume, suspended is
still true; and no KEY_POWER event is send...

Arguably to be consistent we should fix drivers/acpi/button.c to
send KEY_POWER on wakeup too, but that will break things for many
users with a likelyhood of breakage approaching 100%.

Note I'm not telling this to argue against your change, just as
background for why I added this behavior to the axp20x-pek code.

Oh and taking a second look, I see that the hook is already
written so as to only execute on the AXP288 PMIC, which means
it should effectively already only influence X86 machines.

So are you trying to get the KEY_POWER event after wakeup
by power-button to work on a X86 device ?  In that case please
be aware of the drivers/acpi/button.c issue...

I have the feeling that we may need a Kconfig option to configure
whether or not to send KEY_POWER on wakeup by power-button, because
as discussed in my previous mail Android wants this, KDE/MATE/GNOME
not so much...

Regards,

Hans





On x86 axp20x-pek is only used for the power-button on Bay Trail devices
with a AXP288 PMIC. On Cherry Trail devices with an AXP288 PMIC the
power-button is also connected directly to a GPIO on the SoC and that
is used (also see the axp20x_pek_should_register_input function).

So after writing this patch, when doing hw-enablement for the power-button
on the Cherry Trail devices I learned that the gpio_keys driver does
send userspace a KEY_POWER event when woken up with the power-button.

I wrote a patch for gpio-keys to not do this, as that is what normal
Linux userspace expects, but that was nacked, because under e.g.
Android the KEY_POWER event is actually desirable / necessary to avoid
Android immediately re-suspending the system again. Since my "fix" to
the gpio-keys devices was nacked I have instead wroked around this in
userspace, but *only* for the GNOME3 desktop environment, by teaching
GNOME3 to ignore KEY_POWER events for the first couple of seconds after
a resume.

So your suggested change, which will cause KEY_POWER to be send on
Bay Trail devices after a wake-up by the power button, should be
fine for recent GNOME3 versions, but for other desktop environments
this may cause a regression where they respond to the new KEY_POWER
event by immediately going back to sleep again.

As for this not working with the i2c bus, it does on X86 because
the PMIC is also directly accessed by the power-management HW of
the SoC and to make this work the i2c-controller is never suspended
an

[linux-sunxi] Re: [PATCH 3/3] Input: axp20x-pek - Enable wakeup for all AXP variants

2020-01-13 Thread Hans de Goede

Hi,

On 13-01-2020 04:20, Samuel Holland wrote:

There are many devices, including several mobile battery-powered
devices, using other AXP variants as their PMIC. Enable them to use
the power key as a wakeup source.

Signed-off-by: Samuel Holland 



Patch looks good to me:

Reviewed-by: Hans de Goede 

Regards,

Hans


---
  drivers/input/misc/axp20x-pek.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 38cd4a4aeb65..b910c1798e4e 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -279,8 +279,7 @@ static int axp20x_pek_probe_input_device(struct axp20x_pek 
*axp20x_pek,
return error;
}
  
-	if (axp20x_pek->axp20x->variant == AXP288_ID)

-   device_init_wakeup(&pdev->dev, true);
+   device_init_wakeup(&pdev->dev, true);
  
  	return 0;

  }



--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/95bea397-ed7c-2040-a3af-61ded13aca27%40redhat.com.


[linux-sunxi] Re: [PATCH 2/3] Input: axp20x-pek - Respect userspace wakeup configuration

2020-01-13 Thread Hans de Goede

Hi,

On 13-01-2020 04:20, Samuel Holland wrote:

Unlike most other power button drivers, this driver unconditionally
enables its wakeup IRQ. It should be using device_may_wakeup() to
respect the userspace configuration of wakeup sources.

Because the AXP20x MFD device uses regmap-irq, the AXP20x PEK IRQs are
nested off of regmap-irq's threaded interrupt handler. The device core
ignores such interrupts, so to actually disable wakeup, we must
explicitly disable all non-wakeup interrupts during suspend.

Signed-off-by: Samuel Holland 
---
  drivers/input/misc/axp20x-pek.c | 42 -
  1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 7d0ee5bececb..38cd4a4aeb65 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -280,7 +280,7 @@ static int axp20x_pek_probe_input_device(struct axp20x_pek 
*axp20x_pek,
}
  
  	if (axp20x_pek->axp20x->variant == AXP288_ID)

-   enable_irq_wake(axp20x_pek->irq_dbr);
+   device_init_wakeup(&pdev->dev, true);
  
  	return 0;

  }
@@ -352,6 +352,45 @@ static int axp20x_pek_probe(struct platform_device *pdev)
return 0;
  }
  
+#if CONFIG_PM_SLEEP


As the kbuild test robot pointed out, you need to use #ifdef here.

Otherwise this patch looks good to me:

Reviewed-by: Hans de Goede 

Regards,

Hans




+static int axp20x_pek_suspend(struct device *dev)
+{
+   struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+   /*
+* Nested threaded interrupts are not automatically
+* disabled, so we must do it explicitly.
+*/
+   if (device_may_wakeup(dev)) {
+   enable_irq_wake(axp20x_pek->irq_dbf);
+   enable_irq_wake(axp20x_pek->irq_dbr);
+   } else {
+   disable_irq(axp20x_pek->irq_dbf);
+   disable_irq(axp20x_pek->irq_dbr);
+   }
+
+   return 0;
+}
+
+static int axp20x_pek_resume(struct device *dev)
+{
+   struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+   if (device_may_wakeup(dev)) {
+   disable_irq_wake(axp20x_pek->irq_dbf);
+   disable_irq_wake(axp20x_pek->irq_dbr);
+   } else {
+   enable_irq(axp20x_pek->irq_dbf);
+   enable_irq(axp20x_pek->irq_dbr);
+   }
+
+   return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(axp20x_pek_pm_ops, axp20x_pek_suspend,
+   axp20x_pek_resume);
+
  static const struct platform_device_id axp_pek_id_match[] = {
{
.name = "axp20x-pek",
@@ -371,6 +410,7 @@ static struct platform_driver axp20x_pek_driver = {
.driver = {
.name   = "axp20x-pek",
.dev_groups = axp20x_groups,
+   .pm = &axp20x_pek_pm_ops,
},
  };
  module_platform_driver(axp20x_pek_driver);



--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/84e9f44e-81e1-ab3d-3dd0-08388951b074%40redhat.com.


[linux-sunxi] Re: [PATCH 1/3] Input: axp20x-pek - Remove unique wakeup event handling

2020-01-13 Thread Hans de Goede

Hi,

On 13-01-2020 04:20, Samuel Holland wrote:

This driver attempts to avoid reporting wakeup events to userspace by
clearing a possible pending IRQ before IRQs are enabled during resume.
The assumption seems to be that userspace cannot cope with a KEY_POWER
press during resume. However, no other input driver does this, so it
would be a bug that such events are missing with this driver.

Furthermore, for PMICs connected via I2C or RSB, it is not possible to
update the regmap during the noirq resume phase, because the bus
controller drivers require IRQs to perform bus transactions. And the
resume hook cannot move to a later phase, because then it would race
with the power key IRQ handler.

So the best solution seems to be simply removing the hook.


Hmm, I'm not sure this is a good idea, let me give you some background
info on this:

This hook was handled because on X86 systems/laptops when waking
them up typically the power-button does not send a KEY_POWER press event
when the system was woken up through the power-button.

So normal (e.g. Debian, Fedora) userspace does not expect this event
and will directly go to sleep again because that is the default behavior
on a KEY_POWER event.

On x86 axp20x-pek is only used for the power-button on Bay Trail devices
with a AXP288 PMIC. On Cherry Trail devices with an AXP288 PMIC the
power-button is also connected directly to a GPIO on the SoC and that
is used (also see the axp20x_pek_should_register_input function).

So after writing this patch, when doing hw-enablement for the power-button
on the Cherry Trail devices I learned that the gpio_keys driver does
send userspace a KEY_POWER event when woken up with the power-button.

I wrote a patch for gpio-keys to not do this, as that is what normal
Linux userspace expects, but that was nacked, because under e.g.
Android the KEY_POWER event is actually desirable / necessary to avoid
Android immediately re-suspending the system again. Since my "fix" to
the gpio-keys devices was nacked I have instead wroked around this in
userspace, but *only* for the GNOME3 desktop environment, by teaching
GNOME3 to ignore KEY_POWER events for the first couple of seconds after
a resume.

So your suggested change, which will cause KEY_POWER to be send on
Bay Trail devices after a wake-up by the power button, should be
fine for recent GNOME3 versions, but for other desktop environments
this may cause a regression where they respond to the new KEY_POWER
event by immediately going back to sleep again.

As for this not working with the i2c bus, it does on X86 because
the PMIC is also directly accessed by the power-management HW of
the SoC and to make this work the i2c-controller is never suspended
and its irq is marked IRQF_NO_SUSPEND. But this is X86 special
sauce.

Summarizing:

I'm personally fine with remove the magic I added to suppress
the KEY_POWER press reporting as in hindsight given the gpio-keys
story I should have never added it. But I'm worried about this
causing regressions for some Bay Trail users. OTOH making this
change would be good for Android X86 users.

Another IMHO better fix would be to drop the __maybe_unused and
instead wrap both the axp20x_pek_resume_noirq function and the
init of the  .resume_noirq struct member with:

#if defined X86 && defined CONFIG_PM_SLEEP

This keeps the current behavior on Bay Trail machines, while
I assume it should also fix the issues this was causing for
your setup.

Regards,

Hans









Signed-off-by: Samuel Holland 
---
  drivers/input/misc/axp20x-pek.c | 25 -
  1 file changed, 25 deletions(-)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 17c1cca74498..7d0ee5bececb 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -352,30 +352,6 @@ static int axp20x_pek_probe(struct platform_device *pdev)
return 0;
  }
  
-static int __maybe_unused axp20x_pek_resume_noirq(struct device *dev)

-{
-   struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
-
-   if (axp20x_pek->axp20x->variant != AXP288_ID)
-   return 0;
-
-   /*
-* Clear interrupts from button presses during suspend, to avoid
-* a wakeup power-button press getting reported to userspace.
-*/
-   regmap_write(axp20x_pek->axp20x->regmap,
-AXP20X_IRQ1_STATE + AXP288_IRQ_POKN / 8,
-BIT(AXP288_IRQ_POKN % 8));
-
-   return 0;
-}
-
-static const struct dev_pm_ops axp20x_pek_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
-   .resume_noirq = axp20x_pek_resume_noirq,
-#endif
-};
-
  static const struct platform_device_id axp_pek_id_match[] = {
{
.name = "axp20x-pek",
@@ -394,7 +370,6 @@ static struct platform_driver axp20x_pek_driver = {
.id_table   = axp_pek_id_match,
.driver = {
.name   = "axp20x-pek",
-   .pm = &axp20x_pek_pm_ops,
.dev_

[linux-sunxi] Re: [PATCH v6 2/6] mailbox: sun6i-msgbox: Add a new mailbox driver

2020-01-13 Thread Philipp Zabel
On Sun, 2020-01-12 at 23:18 -0600, Samuel Holland wrote:
> Allwinner sun6i, sun8i, sun9i, and sun50i SoCs contain a hardware
> message box used for communication between the ARM CPUs and the ARISC
> management coprocessor. This mailbox contains 8 unidirectional
> 4-message FIFOs.
> 
> Add a driver for it, so it can be used for SCPI or other communication
> protocols.
> 
> Signed-off-by: Samuel Holland 
> ---
>  drivers/mailbox/Kconfig|   9 +
>  drivers/mailbox/Makefile   |   2 +
>  drivers/mailbox/sun6i-msgbox.c | 332 +
>  3 files changed, 343 insertions(+)
>  create mode 100644 drivers/mailbox/sun6i-msgbox.c
> 
[...]
> diff --git a/drivers/mailbox/sun6i-msgbox.c b/drivers/mailbox/sun6i-msgbox.c
> new file mode 100644
> index ..15d6fd522dc5
> --- /dev/null
> +++ b/drivers/mailbox/sun6i-msgbox.c
> @@ -0,0 +1,332 @@
[...]
> + reset = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(reset)) {
> + ret = PTR_ERR(reset);
> + dev_err(dev, "Failed to get reset control: %d\n", ret);
> + goto err_disable_unprepare;
> + }
> +
> + /*
> +  * NOTE: We rely on platform firmware to preconfigure the channel
> +  * directions, and we share this hardware block with other firmware
> +  * that runs concurrently with Linux (e.g. a trusted monitor).
> +  *
> +  * Therefore, we do *not* assert the reset line if probing fails or
> +  * when removing the device.
> +  */
> + ret = reset_control_deassert(reset);
> + if (ret) {
> + dev_err(dev, "Failed to deassert reset: %d\n", ret);
> + goto err_disable_unprepare;
> + }

Reviewed-by: Philipp Zabel 

regards
Philipp

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/a4ff0a8571895c65ed5c20abd0c332b0d92defab.camel%40pengutronix.de.