RE: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH v2 1/1] Drivers: hv: vmbus: Increase wait time for VMbus unload

[Michael Kelley]

From: Vitaly Kuznetsov  Sent: Tuesday, April 20, 2021 2:32 
AM
> 
> Michael Kelley  writes:
> 
> > When running in Azure, disks may be connected to a Linux VM with
> > read/write caching enabled. If a VM panics and issues a VMbus
> > UNLOAD request to Hyper-V, the response is delayed until all dirty
> > data in the disk cache is flushed.  In extreme cases, this flushing
> > can take 10's of seconds, depending on the disk speed and the amount
> > of dirty data. If kdump is configured for the VM, the current 10 second
> > timeout in vmbus_wait_for_unload() may be exceeded, and the UNLOAD
> > complete message may arrive well after the kdump kernel is already
> > running, causing problems.  Note that no problem occurs if kdump is
> > not enabled because Hyper-V waits for the cache flush before doing
> > a reboot through the BIOS/UEFI code.
> >
> > Fix this problem by increasing the timeout in vmbus_wait_for_unload()
> > to 100 seconds. Also output periodic messages so that if anyone is
> > watching the serial console, they won't think the VM is completely
> > hung.
> >
> > Fixes: 911e1987efc8 ("Drivers: hv: vmbus: Add timeout to 
> > vmbus_wait_for_unload")
> > Signed-off-by: Michael Kelley 
> > ---
> >
> > Changed in v2: Fixed silly error in the argument to mdelay()
> >
> > ---
> >  drivers/hv/channel_mgmt.c | 30 +-
> >  1 file changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> > index f3cf4af..ef4685c 100644
> > --- a/drivers/hv/channel_mgmt.c
> > +++ b/drivers/hv/channel_mgmt.c
> > @@ -755,6 +755,12 @@ static void init_vp_index(struct vmbus_channel 
> > *channel)
> > free_cpumask_var(available_mask);
> >  }
> >
> > +#define UNLOAD_DELAY_UNIT_MS   10  /* 10 milliseconds */
> > +#define UNLOAD_WAIT_MS (100*1000)  /* 100 seconds */
> > +#define UNLOAD_WAIT_LOOPS  (UNLOAD_WAIT_MS/UNLOAD_DELAY_UNIT_MS)
> > +#define UNLOAD_MSG_MS  (5*1000)/* Every 5 seconds */
> > +#define UNLOAD_MSG_LOOPS   (UNLOAD_MSG_MS/UNLOAD_DELAY_UNIT_MS)
> > +
> >  static void vmbus_wait_for_unload(void)
> >  {
> > int cpu;
> > @@ -772,12 +778,17 @@ static void vmbus_wait_for_unload(void)
> >  * vmbus_connection.unload_event. If not, the last thing we can do is
> >  * read message pages for all CPUs directly.
> >  *
> > -* Wait no more than 10 seconds so that the panic path can't get
> > -* hung forever in case the response message isn't seen.
> > +* Wait up to 100 seconds since an Azure host must writeback any dirty
> > +* data in its disk cache before the VMbus UNLOAD request will
> > +* complete. This flushing has been empirically observed to take up
> > +* to 50 seconds in cases with a lot of dirty data, so allow additional
> > +* leeway and for inaccuracies in mdelay(). But eventually time out so
> > +* that the panic path can't get hung forever in case the response
> > +* message isn't seen.
> 
> I vaguely remember debugging cases when CHANNELMSG_UNLOAD_RESPONSE never
> arrives, it was kind of pointless to proceed to kexec as attempts to
> reconnect Vmbus devices were failing (no devices were offered after
> CHANNELMSG_REQUESTOFFERS AFAIR). Would it maybe make sense to just do
> emergency reboot instead of proceeding to kexec when this happens? Just
> wondering.
> 

Yes, I think there have been (and maybe still are) situations where we don't
ever get the UNLOAD response.  But there have been bugs fixed in Hyper-V
that I think make that less likely.  There's also an unfixed (and maybe not 
fixable)
problem when not operating in STIMER Direct Mode, where an old-style
timer message can block the UNLOAD response message.  But as the world
moves forward to later kernel versions that use STIMER Direct Mode, that
also becomes less likely.   So my inclination is to let execution continue on
the normal execution path, even if the UNLOAD response message isn't
received.  Maybe we just didn't wait quite long enough (even at 100 seconds).
It's a judgment call, and it's not clear to me that doing an emergency reboot
is really any better.

As background work for this patch, we also discovered another bug in Hyper-V.
If the kdump kernel runs and does a VMbus INITIATE_CONTACT while the
UNLOAD is still in progress, the Hyper-V code is supposed to wait for the UNLOAD
to complete, and then commence the VMbus version negotiation.  But it
doesn't do that -- it finally sends the UNLOAD response, but never does the
version negotiation, so the kdump kernel hangs forever.  The Hyper-V team
plans to fix this, and hopefully we'll get a patch deployed in Azure, which
will eliminate one more scenario where the kdump kernel doesn't succeed.

Michael

> >  */
> > -   for (i = 0; i < 1000; i++) {
> > +   for (i = 1; i <= UNLOAD_WAIT_LOOPS; i++) {
> > if (completion_done(_connection.unload_event))
> > -   break;
> > + 

Re: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH v2 1/1] Drivers: hv: vmbus: Increase wait time for VMbus unload

[Wei Liu]

On Tue, Apr 20, 2021 at 11:31:54AM +0200, Vitaly Kuznetsov wrote:
> Michael Kelley  writes:
> 
> > When running in Azure, disks may be connected to a Linux VM with
> > read/write caching enabled. If a VM panics and issues a VMbus
> > UNLOAD request to Hyper-V, the response is delayed until all dirty
> > data in the disk cache is flushed.  In extreme cases, this flushing
> > can take 10's of seconds, depending on the disk speed and the amount
> > of dirty data. If kdump is configured for the VM, the current 10 second
> > timeout in vmbus_wait_for_unload() may be exceeded, and the UNLOAD
> > complete message may arrive well after the kdump kernel is already
> > running, causing problems.  Note that no problem occurs if kdump is
> > not enabled because Hyper-V waits for the cache flush before doing
> > a reboot through the BIOS/UEFI code.
> >
> > Fix this problem by increasing the timeout in vmbus_wait_for_unload()
> > to 100 seconds. Also output periodic messages so that if anyone is
> > watching the serial console, they won't think the VM is completely
> > hung.
> >
> > Fixes: 911e1987efc8 ("Drivers: hv: vmbus: Add timeout to 
> > vmbus_wait_for_unload")
> > Signed-off-by: Michael Kelley 

Applied to hyperv-next. Thanks.

> > ---
[...]
> >  
> > +#define UNLOAD_DELAY_UNIT_MS   10  /* 10 milliseconds */
> > +#define UNLOAD_WAIT_MS (100*1000)  /* 100 seconds */
> > +#define UNLOAD_WAIT_LOOPS  (UNLOAD_WAIT_MS/UNLOAD_DELAY_UNIT_MS)
> > +#define UNLOAD_MSG_MS  (5*1000)/* Every 5 seconds */
> > +#define UNLOAD_MSG_LOOPS   (UNLOAD_MSG_MS/UNLOAD_DELAY_UNIT_MS)
> > +
> >  static void vmbus_wait_for_unload(void)
> >  {
> > int cpu;
> > @@ -772,12 +778,17 @@ static void vmbus_wait_for_unload(void)
> >  * vmbus_connection.unload_event. If not, the last thing we can do is
> >  * read message pages for all CPUs directly.
> >  *
> > -* Wait no more than 10 seconds so that the panic path can't get
> > -* hung forever in case the response message isn't seen.
> > +* Wait up to 100 seconds since an Azure host must writeback any dirty
> > +* data in its disk cache before the VMbus UNLOAD request will
> > +* complete. This flushing has been empirically observed to take up
> > +* to 50 seconds in cases with a lot of dirty data, so allow additional
> > +* leeway and for inaccuracies in mdelay(). But eventually time out so
> > +* that the panic path can't get hung forever in case the response
> > +* message isn't seen.
> 
> I vaguely remember debugging cases when CHANNELMSG_UNLOAD_RESPONSE never
> arrives, it was kind of pointless to proceed to kexec as attempts to
> reconnect Vmbus devices were failing (no devices were offered after
> CHANNELMSG_REQUESTOFFERS AFAIR). Would it maybe make sense to just do
> emergency reboot instead of proceeding to kexec when this happens? Just
> wondering.
> 

Please submit a follow-up patch if necessary.

Wei.

Re: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH v2 1/1] Drivers: hv: vmbus: Increase wait time for VMbus unload

[Vitaly Kuznetsov]

Michael Kelley  writes:

> When running in Azure, disks may be connected to a Linux VM with
> read/write caching enabled. If a VM panics and issues a VMbus
> UNLOAD request to Hyper-V, the response is delayed until all dirty
> data in the disk cache is flushed.  In extreme cases, this flushing
> can take 10's of seconds, depending on the disk speed and the amount
> of dirty data. If kdump is configured for the VM, the current 10 second
> timeout in vmbus_wait_for_unload() may be exceeded, and the UNLOAD
> complete message may arrive well after the kdump kernel is already
> running, causing problems.  Note that no problem occurs if kdump is
> not enabled because Hyper-V waits for the cache flush before doing
> a reboot through the BIOS/UEFI code.
>
> Fix this problem by increasing the timeout in vmbus_wait_for_unload()
> to 100 seconds. Also output periodic messages so that if anyone is
> watching the serial console, they won't think the VM is completely
> hung.
>
> Fixes: 911e1987efc8 ("Drivers: hv: vmbus: Add timeout to 
> vmbus_wait_for_unload")
> Signed-off-by: Michael Kelley 
> ---
>
> Changed in v2: Fixed silly error in the argument to mdelay()
>
> ---
>  drivers/hv/channel_mgmt.c | 30 +-
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index f3cf4af..ef4685c 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -755,6 +755,12 @@ static void init_vp_index(struct vmbus_channel *channel)
>   free_cpumask_var(available_mask);
>  }
>  
> +#define UNLOAD_DELAY_UNIT_MS 10  /* 10 milliseconds */
> +#define UNLOAD_WAIT_MS   (100*1000)  /* 100 seconds */
> +#define UNLOAD_WAIT_LOOPS(UNLOAD_WAIT_MS/UNLOAD_DELAY_UNIT_MS)
> +#define UNLOAD_MSG_MS(5*1000)/* Every 5 seconds */
> +#define UNLOAD_MSG_LOOPS (UNLOAD_MSG_MS/UNLOAD_DELAY_UNIT_MS)
> +
>  static void vmbus_wait_for_unload(void)
>  {
>   int cpu;
> @@ -772,12 +778,17 @@ static void vmbus_wait_for_unload(void)
>* vmbus_connection.unload_event. If not, the last thing we can do is
>* read message pages for all CPUs directly.
>*
> -  * Wait no more than 10 seconds so that the panic path can't get
> -  * hung forever in case the response message isn't seen.
> +  * Wait up to 100 seconds since an Azure host must writeback any dirty
> +  * data in its disk cache before the VMbus UNLOAD request will
> +  * complete. This flushing has been empirically observed to take up
> +  * to 50 seconds in cases with a lot of dirty data, so allow additional
> +  * leeway and for inaccuracies in mdelay(). But eventually time out so
> +  * that the panic path can't get hung forever in case the response
> +  * message isn't seen.

I vaguely remember debugging cases when CHANNELMSG_UNLOAD_RESPONSE never
arrives, it was kind of pointless to proceed to kexec as attempts to
reconnect Vmbus devices were failing (no devices were offered after
CHANNELMSG_REQUESTOFFERS AFAIR). Would it maybe make sense to just do
emergency reboot instead of proceeding to kexec when this happens? Just
wondering.

>*/
> - for (i = 0; i < 1000; i++) {
> + for (i = 1; i <= UNLOAD_WAIT_LOOPS; i++) {
>   if (completion_done(_connection.unload_event))
> - break;
> + goto completed;
>  
>   for_each_online_cpu(cpu) {
>   struct hv_per_cpu_context *hv_cpu
> @@ -800,9 +811,18 @@ static void vmbus_wait_for_unload(void)
>   vmbus_signal_eom(msg, message_type);
>   }
>  
> - mdelay(10);
> + /*
> +  * Give a notice periodically so someone watching the
> +  * serial output won't think it is completely hung.
> +  */
> + if (!(i % UNLOAD_MSG_LOOPS))
> + pr_notice("Waiting for VMBus UNLOAD to complete\n");
> +
> + mdelay(UNLOAD_DELAY_UNIT_MS);
>   }
> + pr_err("Continuing even though VMBus UNLOAD did not complete\n");
>  
> +completed:
>   /*
>* We're crashing and already got the UNLOAD_RESPONSE, cleanup all
>* maybe-pending messages on all CPUs to be able to receive new

This is definitely an improvement,

Reviewed-by: Vitaly Kuznetsov 

-- 
Vitaly

[PATCH v3 2/2] i2c: stm32f7: add SMBus-Alert support

[Alain Volmat]

Add support for the SMBus-Alert protocol to the STM32F7 that has
dedicated control and status logic.

If SMBus-Alert is used, the SMBALERT# pin must be configured as alternate
function for I2C Alert.

Signed-off-by: Alain Volmat 
Reviewed-by: Pierre-Yves MORDRET 

---
v2: - rely on st,smbus-alert binding instead of smbus
---
 drivers/i2c/busses/i2c-stm32f7.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index c62c815b88eb..bd840cd2b9e4 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -51,6 +51,7 @@
 
 /* STM32F7 I2C control 1 */
 #define STM32F7_I2C_CR1_PECEN  BIT(23)
+#define STM32F7_I2C_CR1_ALERTENBIT(22)
 #define STM32F7_I2C_CR1_SMBHEN BIT(20)
 #define STM32F7_I2C_CR1_WUPEN  BIT(18)
 #define STM32F7_I2C_CR1_SBCBIT(16)
@@ -125,6 +126,7 @@
(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
 #define STM32F7_I2C_ISR_DIRBIT(16)
 #define STM32F7_I2C_ISR_BUSY   BIT(15)
+#define STM32F7_I2C_ISR_ALERT  BIT(13)
 #define STM32F7_I2C_ISR_PECERR BIT(11)
 #define STM32F7_I2C_ISR_ARLO   BIT(9)
 #define STM32F7_I2C_ISR_BERR   BIT(8)
@@ -138,6 +140,7 @@
 #define STM32F7_I2C_ISR_TXEBIT(0)
 
 /* STM32F7 I2C Interrupt Clear */
+#define STM32F7_I2C_ICR_ALERTCFBIT(13)
 #define STM32F7_I2C_ICR_PECCF  BIT(11)
 #define STM32F7_I2C_ICR_ARLOCF BIT(9)
 #define STM32F7_I2C_ICR_BERRCF BIT(8)
@@ -283,6 +286,17 @@ struct stm32f7_i2c_msg {
u8 smbus_buf[I2C_SMBUS_BLOCK_MAX + 3] __aligned(4);
 };
 
+/**
+ * struct stm32f7_i2c_alert - SMBus alert specific data
+ * @setup: platform data for the smbus_alert i2c client
+ * @ara: I2C slave device used to respond to the SMBus Alert with Alert
+ * Response Address
+ */
+struct stm32f7_i2c_alert {
+   struct i2c_smbus_alert_setup setup;
+   struct i2c_client *ara;
+};
+
 /**
  * struct stm32f7_i2c_dev - private data of the controller
  * @adap: I2C adapter for this controller
@@ -312,6 +326,7 @@ struct stm32f7_i2c_msg {
  * @wakeup_src: boolean to know if the device is a wakeup source
  * @smbus_mode: states that the controller is configured in SMBus mode
  * @host_notify_client: SMBus host-notify client
+ * @alert: SMBus alert specific data
  */
 struct stm32f7_i2c_dev {
struct i2c_adapter adap;
@@ -340,6 +355,7 @@ struct stm32f7_i2c_dev {
bool wakeup_src;
bool smbus_mode;
struct i2c_client *host_notify_client;
+   struct stm32f7_i2c_alert *alert;
 };
 
 /*
@@ -1616,6 +1632,13 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
*data)
f7_msg->result = -EINVAL;
}
 
+   if (status & STM32F7_I2C_ISR_ALERT) {
+   dev_dbg(dev, "<%s>: SMBus alert received\n", __func__);
+   writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
+   i2c_handle_smbus_alert(i2c_dev->alert->ara);
+   return IRQ_HANDLED;
+   }
+
if (!i2c_dev->slave_running) {
u32 mask;
/* Disable interrupts */
@@ -1982,6 +2005,42 @@ static void stm32f7_i2c_disable_smbus_host(struct 
stm32f7_i2c_dev *i2c_dev)
}
 }
 
+static int stm32f7_i2c_enable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
+{
+   struct stm32f7_i2c_alert *alert;
+   struct i2c_adapter *adap = _dev->adap;
+   struct device *dev = i2c_dev->dev;
+   void __iomem *base = i2c_dev->base;
+
+   alert = devm_kzalloc(dev, sizeof(*alert), GFP_KERNEL);
+   if (!alert)
+   return -ENOMEM;
+
+   alert->ara = i2c_new_smbus_alert_device(adap, >setup);
+   if (IS_ERR(alert->ara))
+       return PTR_ERR(alert->ara);
+
+   i2c_dev->alert = alert;
+
+   /* Enable SMBus Alert */
+   stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, STM32F7_I2C_CR1_ALERTEN);
+
+   return 0;
+}
+
+static void stm32f7_i2c_disable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
+{
+   struct stm32f7_i2c_alert *alert = i2c_dev->alert;
+   void __iomem *base = i2c_dev->base;
+
+   if (alert) {
+   /* Disable SMBus Alert */
+   stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1,
+    STM32F7_I2C_CR1_ALERTEN);
+   i2c_unregister_device(alert->ara);
+   }
+}
+
 static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
 {
struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
@@ -2169,6 +2228,16 @@ static int stm32f7_i2c_probe(struct platform_device 
*pdev)
}
    }
 
+   if (of_property_read_bool(pdev->dev.of_node, &qu

[PATCH v3 1/2] dt-bindings: i2c: stm32f7: add st,smbus-alert binding for SMBus Alert

[Alain Volmat]

Based on the SMBus specification, SMBus Alert active state is low.
As often on SoC, the SMBus Alert pin is not only dedicated to this
feature and can also be used for another purpose (by configuring it
as alternate function for other functions via pinctrl).

"smbus" dt-binding has been introduced recently [1], however it is also
used to indicate usage of host-notify feature.
Relying on 'smbus' binding for SMBus-Alert as well as it was discussed
previously [2] would lead to requiring the SMBALERT# pin to be configured
as alternate function for i2c/smbus controller even if only host-notify is
needed.
Indeed, not doing so would lead to spurious SMBus Alert interrupts since
the i2c/smbus controller would see the (not configured) SMBA pin as low
level.

For that reason, SMBus-Alert needs to have its own binding in order
to only be enabled whenever SMBALERT# pin is configured as alternate
function for i2c/smbus controller.

[1] https://lore.kernel.org/linux-i2c/20200721062217.GA1044@kunai/
[2] 
https://lore.kernel.org/linux-i2c/20200701143738.gf3...@gnbcxd0016.gnb.st.com/

Signed-off-by: Alain Volmat 
Reviewed-by: Rob Herring 

---
v3: use lore.kernel.org links
v2: introduce st,smbus-alert property
---
 Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
index d747f4990ad8..0d45ead7d835 100644
--- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
@@ -36,6 +36,11 @@ allOf:
 minItems: 3
 maxItems: 3
 
+    st,smbus-alert:
+  description: Enable the SMBus-Alert via SMBA pin, note SMBA pin
+   must also be configured via pinctrl.
+  type: boolean
+
   - if:
   properties:
 compatible:
-- 
2.17.1

[PATCH v3 0/2] i2c: stm32f7: add SMBus-Alert support

[Alain Volmat]

This serie adds support for SMBus Alert on the STM32F7.
A new binding st,smbus-alert is added in order to differenciate
with the existing smbus binding.

SMBA alert control and status logic must be enabled along with
SMBALERT# pin configured via pinctrl in the device tree. This is the
rational for adding "st,smbus-alert" property.

---
v3:
use lore.kernel.org links instead of marc.info

v2:
When SMBUS alert isn't available on the board (SMBA unused), this
logic musn't be enabled. Enabling it unconditionally wrongly lead to get
SMBA interrupts.
So, add "st,smbus-alert" dedicated binding to have a smbus alert with a
consistent pin configuration in DT.

Alain Volmat (2):
  dt-bindings: i2c: stm32f7: add st,smbus-alert binding for SMBus Alert
  i2c: stm32f7: add SMBus-Alert support

 .../devicetree/bindings/i2c/st,stm32-i2c.yaml |  5 ++
 drivers/i2c/busses/i2c-stm32f7.c  | 73 +++
 2 files changed, 78 insertions(+)

-- 
2.17.1

Re: [PATCH v2 1/2] dt-bindings: i2c: stm32f7: add st,smbus-alert binding for SMBus Alert

[Rob Herring]

On Thu, Mar 18, 2021 at 02:44:48PM +0100, Alain Volmat wrote:
> Based on the SMBus specification, SMBus Alert active state is low.
> As often on SoC, the SMBus Alert pin is not only dedicated to this
> feature and can also be used for another purpose (by configuring it
> as alternate function for other functions via pinctrl).
> 
> "smbus" dt-binding has been introduced recently [1], however it is also
> used to indicate usage of host-notify feature.
> Relying on 'smbus' binding for SMBus-Alert as well as it was discussed
> previously [2] would lead to requiring the SMBALERT# pin to be configured
> as alternate function for i2c/smbus controller even if only host-notify is
> needed.
> Indeed, not doing so would lead to spurious SMBus Alert interrupts since
> the i2c/smbus controller would see the (not configured) SMBA pin as low
> level.
> 
> For that reason, SMBus-Alert needs to have its own binding in order
> to only be enabled whenever SMBALERT# pin is configured as alternate
> function for i2c/smbus controller.
> 
> [1] https://marc.info/?l=linux-i2c=159531254413805=2
> [2] https://marc.info/?l=linux-renesas-soc=159361426409817=2

Please use lore.kernel.org links.

> 
> Signed-off-by: Alain Volmat 
> 
> ---
> v2: introduce st,smbus-alert property
> ---
>  Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 5 +
>  1 file changed, 5 insertions(+)

With that,

Reviewed-by: Rob Herring 

Though I defer to Wolfram whether this could/should be common instead.

Rob

Re: [PATCH v2 2/2] i2c: stm32f7: add SMBus-Alert support

[Pierre Yves MORDRET]

Hi All

On 3/18/21 2:44 PM, Alain Volmat wrote:
> Add support for the SMBus-Alert protocol to the STM32F7 that has
> dedicated control and status logic.
> 
> If SMBus-Alert is used, the SMBALERT# pin must be configured as alternate
> function for I2C Alert.
> 
> Signed-off-by: Alain Volmat 
> 
> ---
> v2: - rely on st,smbus-alert binding instead of smbus
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 73 
>  1 file changed, 73 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c 
> b/drivers/i2c/busses/i2c-stm32f7.c
> index c62c815b88eb..bd840cd2b9e4 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -51,6 +51,7 @@
>  
>  /* STM32F7 I2C control 1 */
>  #define STM32F7_I2C_CR1_PECENBIT(23)
> +#define STM32F7_I2C_CR1_ALERTEN  BIT(22)
>  #define STM32F7_I2C_CR1_SMBHEN   BIT(20)
>  #define STM32F7_I2C_CR1_WUPENBIT(18)
>  #define STM32F7_I2C_CR1_SBC  BIT(16)
> @@ -125,6 +126,7 @@
>   (((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
>  #define STM32F7_I2C_ISR_DIR  BIT(16)
>  #define STM32F7_I2C_ISR_BUSY BIT(15)
> +#define STM32F7_I2C_ISR_ALERTBIT(13)
>  #define STM32F7_I2C_ISR_PECERR   BIT(11)
>  #define STM32F7_I2C_ISR_ARLO BIT(9)
>  #define STM32F7_I2C_ISR_BERR BIT(8)
> @@ -138,6 +140,7 @@
>  #define STM32F7_I2C_ISR_TXE  BIT(0)
>  
>  /* STM32F7 I2C Interrupt Clear */
> +#define STM32F7_I2C_ICR_ALERTCF  BIT(13)
>  #define STM32F7_I2C_ICR_PECCFBIT(11)
>  #define STM32F7_I2C_ICR_ARLOCF   BIT(9)
>  #define STM32F7_I2C_ICR_BERRCF   BIT(8)
> @@ -283,6 +286,17 @@ struct stm32f7_i2c_msg {
>   u8 smbus_buf[I2C_SMBUS_BLOCK_MAX + 3] __aligned(4);
>  };
>  
> +/**
> + * struct stm32f7_i2c_alert - SMBus alert specific data
> + * @setup: platform data for the smbus_alert i2c client
> + * @ara: I2C slave device used to respond to the SMBus Alert with Alert
> + * Response Address
> + */
> +struct stm32f7_i2c_alert {
> + struct i2c_smbus_alert_setup setup;
> + struct i2c_client *ara;
> +};
> +
>  /**
>   * struct stm32f7_i2c_dev - private data of the controller
>   * @adap: I2C adapter for this controller
> @@ -312,6 +326,7 @@ struct stm32f7_i2c_msg {
>   * @wakeup_src: boolean to know if the device is a wakeup source
>   * @smbus_mode: states that the controller is configured in SMBus mode
>   * @host_notify_client: SMBus host-notify client
> + * @alert: SMBus alert specific data
>   */
>  struct stm32f7_i2c_dev {
>   struct i2c_adapter adap;
> @@ -340,6 +355,7 @@ struct stm32f7_i2c_dev {
>   bool wakeup_src;
>   bool smbus_mode;
>   struct i2c_client *host_notify_client;
> + struct stm32f7_i2c_alert *alert;
>  };
>  
>  /*
> @@ -1616,6 +1632,13 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
> *data)
>   f7_msg->result = -EINVAL;
>   }
>  
> + if (status & STM32F7_I2C_ISR_ALERT) {
> + dev_dbg(dev, "<%s>: SMBus alert received\n", __func__);
> + writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
> + i2c_handle_smbus_alert(i2c_dev->alert->ara);
> + return IRQ_HANDLED;
> + }
> +
>   if (!i2c_dev->slave_running) {
>   u32 mask;
>   /* Disable interrupts */
> @@ -1982,6 +2005,42 @@ static void stm32f7_i2c_disable_smbus_host(struct 
> stm32f7_i2c_dev *i2c_dev)
>   }
>  }
>  
> +static int stm32f7_i2c_enable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
> +{
> + struct stm32f7_i2c_alert *alert;
> + struct i2c_adapter *adap = _dev->adap;
> + struct device *dev = i2c_dev->dev;
> + void __iomem *base = i2c_dev->base;
> +
> + alert = devm_kzalloc(dev, sizeof(*alert), GFP_KERNEL);
> + if (!alert)
> + return -ENOMEM;
> +
> + alert->ara = i2c_new_smbus_alert_device(adap, >setup);
> + if (IS_ERR(alert->ara))
> + return PTR_ERR(alert->ara);
> +
> + i2c_dev->alert = alert;
> +
> + /* Enable SMBus Alert */
> + stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, STM32F7_I2C_CR1_ALERTEN);
> +
> + return 0;
> +}
> +
> +static void stm32f7_i2c_disable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
> +{
> + struct stm32f7_i2c_alert *alert = i2c_dev->alert;
> + void __iomem *b

[PATCH v2 0/2] i2c: stm32f7: add SMBus-Alert support

[Alain Volmat]

This serie adds support for SMBus Alert on the STM32F7.
A new binding st,smbus-alert is added in order to differenciate
with the existing smbus binding.

SMBA alert control and status logic must be enabled along with
SMBALERT# pin configured via pinctrl in the device tree. This is the
rational for adding "st,smbus-alert" property.

---
v2:
When SMBUS alert isn't available on the board (SMBA unused), this
logic musn't be enabled. Enabling it unconditionally wrongly lead to get
SMBA interrupts.
So, add "st,smbus-alert" dedicated binding to have a smbus alert with a
consistent pin configuration in DT.

Alain Volmat (2):
  dt-bindings: i2c: stm32f7: add st,smbus-alert binding for SMBus Alert
  i2c: stm32f7: add SMBus-Alert support

 .../devicetree/bindings/i2c/st,stm32-i2c.yaml |  5 ++
 drivers/i2c/busses/i2c-stm32f7.c  | 73 +++
 2 files changed, 78 insertions(+)

-- 
2.17.1

[PATCH v2 2/2] i2c: stm32f7: add SMBus-Alert support

[Alain Volmat]

Add support for the SMBus-Alert protocol to the STM32F7 that has
dedicated control and status logic.

If SMBus-Alert is used, the SMBALERT# pin must be configured as alternate
function for I2C Alert.

Signed-off-by: Alain Volmat 

---
v2: - rely on st,smbus-alert binding instead of smbus
---
 drivers/i2c/busses/i2c-stm32f7.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index c62c815b88eb..bd840cd2b9e4 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -51,6 +51,7 @@
 
 /* STM32F7 I2C control 1 */
 #define STM32F7_I2C_CR1_PECEN  BIT(23)
+#define STM32F7_I2C_CR1_ALERTENBIT(22)
 #define STM32F7_I2C_CR1_SMBHEN BIT(20)
 #define STM32F7_I2C_CR1_WUPEN  BIT(18)
 #define STM32F7_I2C_CR1_SBCBIT(16)
@@ -125,6 +126,7 @@
(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
 #define STM32F7_I2C_ISR_DIRBIT(16)
 #define STM32F7_I2C_ISR_BUSY   BIT(15)
+#define STM32F7_I2C_ISR_ALERT  BIT(13)
 #define STM32F7_I2C_ISR_PECERR BIT(11)
 #define STM32F7_I2C_ISR_ARLO   BIT(9)
 #define STM32F7_I2C_ISR_BERR   BIT(8)
@@ -138,6 +140,7 @@
 #define STM32F7_I2C_ISR_TXEBIT(0)
 
 /* STM32F7 I2C Interrupt Clear */
+#define STM32F7_I2C_ICR_ALERTCFBIT(13)
 #define STM32F7_I2C_ICR_PECCF  BIT(11)
 #define STM32F7_I2C_ICR_ARLOCF BIT(9)
 #define STM32F7_I2C_ICR_BERRCF BIT(8)
@@ -283,6 +286,17 @@ struct stm32f7_i2c_msg {
u8 smbus_buf[I2C_SMBUS_BLOCK_MAX + 3] __aligned(4);
 };
 
+/**
+ * struct stm32f7_i2c_alert - SMBus alert specific data
+ * @setup: platform data for the smbus_alert i2c client
+ * @ara: I2C slave device used to respond to the SMBus Alert with Alert
+ * Response Address
+ */
+struct stm32f7_i2c_alert {
+   struct i2c_smbus_alert_setup setup;
+   struct i2c_client *ara;
+};
+
 /**
  * struct stm32f7_i2c_dev - private data of the controller
  * @adap: I2C adapter for this controller
@@ -312,6 +326,7 @@ struct stm32f7_i2c_msg {
  * @wakeup_src: boolean to know if the device is a wakeup source
  * @smbus_mode: states that the controller is configured in SMBus mode
  * @host_notify_client: SMBus host-notify client
+ * @alert: SMBus alert specific data
  */
 struct stm32f7_i2c_dev {
struct i2c_adapter adap;
@@ -340,6 +355,7 @@ struct stm32f7_i2c_dev {
bool wakeup_src;
bool smbus_mode;
struct i2c_client *host_notify_client;
+   struct stm32f7_i2c_alert *alert;
 };
 
 /*
@@ -1616,6 +1632,13 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
*data)
f7_msg->result = -EINVAL;
}
 
+   if (status & STM32F7_I2C_ISR_ALERT) {
+   dev_dbg(dev, "<%s>: SMBus alert received\n", __func__);
+   writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
+   i2c_handle_smbus_alert(i2c_dev->alert->ara);
+   return IRQ_HANDLED;
+   }
+
if (!i2c_dev->slave_running) {
u32 mask;
/* Disable interrupts */
@@ -1982,6 +2005,42 @@ static void stm32f7_i2c_disable_smbus_host(struct 
stm32f7_i2c_dev *i2c_dev)
}
 }
 
+static int stm32f7_i2c_enable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
+{
+   struct stm32f7_i2c_alert *alert;
+   struct i2c_adapter *adap = _dev->adap;
+   struct device *dev = i2c_dev->dev;
+   void __iomem *base = i2c_dev->base;
+
+   alert = devm_kzalloc(dev, sizeof(*alert), GFP_KERNEL);
+   if (!alert)
+   return -ENOMEM;
+
+   alert->ara = i2c_new_smbus_alert_device(adap, >setup);
+   if (IS_ERR(alert->ara))
+       return PTR_ERR(alert->ara);
+
+   i2c_dev->alert = alert;
+
+   /* Enable SMBus Alert */
+   stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, STM32F7_I2C_CR1_ALERTEN);
+
+   return 0;
+}
+
+static void stm32f7_i2c_disable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
+{
+   struct stm32f7_i2c_alert *alert = i2c_dev->alert;
+   void __iomem *base = i2c_dev->base;
+
+   if (alert) {
+   /* Disable SMBus Alert */
+   stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1,
+    STM32F7_I2C_CR1_ALERTEN);
+   i2c_unregister_device(alert->ara);
+   }
+}
+
 static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
 {
struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
@@ -2169,6 +2228,16 @@ static int stm32f7_i2c_probe(struct platform_device 
*pdev)
}
    }
 
+   if (of_property_read_bool(pdev->dev.of_node, "st,smbus-alert")) 

[PATCH v2 1/2] dt-bindings: i2c: stm32f7: add st,smbus-alert binding for SMBus Alert

[Alain Volmat]

Based on the SMBus specification, SMBus Alert active state is low.
As often on SoC, the SMBus Alert pin is not only dedicated to this
feature and can also be used for another purpose (by configuring it
as alternate function for other functions via pinctrl).

"smbus" dt-binding has been introduced recently [1], however it is also
used to indicate usage of host-notify feature.
Relying on 'smbus' binding for SMBus-Alert as well as it was discussed
previously [2] would lead to requiring the SMBALERT# pin to be configured
as alternate function for i2c/smbus controller even if only host-notify is
needed.
Indeed, not doing so would lead to spurious SMBus Alert interrupts since
the i2c/smbus controller would see the (not configured) SMBA pin as low
level.

For that reason, SMBus-Alert needs to have its own binding in order
to only be enabled whenever SMBALERT# pin is configured as alternate
function for i2c/smbus controller.

[1] https://marc.info/?l=linux-i2c=159531254413805=2
[2] https://marc.info/?l=linux-renesas-soc=159361426409817=2

Signed-off-by: Alain Volmat 

---
v2: introduce st,smbus-alert property
---
 Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
index d747f4990ad8..0d45ead7d835 100644
--- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
@@ -36,6 +36,11 @@ allOf:
 minItems: 3
 maxItems: 3
 
+    st,smbus-alert:
+  description: Enable the SMBus-Alert via SMBA pin, note SMBA pin
+   must also be configured via pinctrl.
+  type: boolean
+
   - if:
   properties:
 compatible:
-- 
2.17.1

RE: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH v2 01/10] Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral code

[Michael Kelley]

From: Vitaly Kuznetsov  Sent: Tuesday, March 2, 2021 4:57 
AM
> 
> Michael Kelley  writes:
> 
> > The Hyper-V page allocator functions are implemented in an architecture
> > neutral way.  Move them into the architecture neutral VMbus module so
> > a separate implementation for ARM64 is not needed.
> >
> > No functional change.
> >
> > Signed-off-by: Michael Kelley 
> > Reviewed-by: Boqun Feng 
> > ---
> >  arch/x86/hyperv/hv_init.c   | 22 --
> >  arch/x86/include/asm/mshyperv.h |  5 -
> >  drivers/hv/hv.c | 36 
> >  include/asm-generic/mshyperv.h  |  4 
> >  4 files changed, 40 insertions(+), 27 deletions(-)
> >

[snip]

> >
> >  /*
> > + * Functions for allocating and freeing memory with size and
> > + * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> > + * the guest page size may not be the same as the Hyper-V page
> > + * size. We depend upon kmalloc() aligning power-of-two size
> > + * allocations to the allocation size boundary, so that the
> > + * allocated memory appears to Hyper-V as a page of the size
> > + * it expects.
> > + */
> > +
> > +void *hv_alloc_hyperv_page(void)
> > +{
> > +   BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> > +
> > +   if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > +   return (void *)__get_free_page(GFP_KERNEL);
> > +   else
> > +   return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> 
> PAGE_SIZE and HV_HYP_PAGE_SIZE are known compile-time and in case this
> won't change in the future we can probably write this as
> 
> #if PAGE_SIZE == HV_HYP_PAGE_SIZE
>return (void *)__get_free_page(GFP_KERNEL);
> #else
>return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> #endif
> 
> (not sure if the output is going to be any different with e.g. gcc's '-O2')
> 

I looked at the generated code, and the compiler does the right
thing on both x86/x64 and on ARM64.  I'd rather leave the code
as is so that both legs of the 'if' statement get checked by the
compiler regardless of whether PAGE_SIZE == HV_HYP_PAGE_SIZE.

Michael

> > +}
> > +
> > +void *hv_alloc_hyperv_zeroed_page(void)
> > +{
> > +   if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > +   return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > +   else
> > +   return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > +}
> > +
> > +void hv_free_hyperv_page(unsigned long addr)
> > +{
> > +   if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > +   free_page(addr);
> > +   else
> > +   kfree((void *)addr);
> > +}
> > +
> > +/*

Re: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH v2 01/10] Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral code

[Vitaly Kuznetsov]

Michael Kelley  writes:

> The Hyper-V page allocator functions are implemented in an architecture
> neutral way.  Move them into the architecture neutral VMbus module so
> a separate implementation for ARM64 is not needed.
>
> No functional change.
>
> Signed-off-by: Michael Kelley 
> Reviewed-by: Boqun Feng 
> ---
>  arch/x86/hyperv/hv_init.c   | 22 --
>  arch/x86/include/asm/mshyperv.h |  5 -
>  drivers/hv/hv.c | 36 
>  include/asm-generic/mshyperv.h  |  4 
>  4 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index b81047d..4bdb344 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -54,28 +54,6 @@
>  u32 hv_max_vp_index;
>  EXPORT_SYMBOL_GPL(hv_max_vp_index);
>  
> -void *hv_alloc_hyperv_page(void)
> -{
> - BUILD_BUG_ON(PAGE_SIZE != HV_HYP_PAGE_SIZE);
> -
> - return (void *)__get_free_page(GFP_KERNEL);
> -}
> -EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> -
> -void *hv_alloc_hyperv_zeroed_page(void)
> -{
> -BUILD_BUG_ON(PAGE_SIZE != HV_HYP_PAGE_SIZE);
> -
> -return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> -}
> -EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
> -
> -void hv_free_hyperv_page(unsigned long addr)
> -{
> - free_page(addr);
> -}
> -EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
> -
>  static int hv_cpu_init(unsigned int cpu)
>  {
>   u64 msr_vp_index;
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index ccf60a8..ef6e968 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -233,9 +233,6 @@ static inline struct hv_vp_assist_page 
> *hv_get_vp_assist_page(unsigned int cpu)
>  
>  void __init hyperv_init(void);
>  void hyperv_setup_mmu_ops(void);
> -void *hv_alloc_hyperv_page(void);
> -void *hv_alloc_hyperv_zeroed_page(void);
> -void hv_free_hyperv_page(unsigned long addr);
>  void set_hv_tscchange_cb(void (*cb)(void));
>  void clear_hv_tscchange_cb(void);
>  void hyperv_stop_tsc_emulation(void);
> @@ -272,8 +269,6 @@ int hv_map_ioapic_interrupt(int ioapic_id, bool level, 
> int vcpu, int vector,
>  #else /* CONFIG_HYPERV */
>  static inline void hyperv_init(void) {}
>  static inline void hyperv_setup_mmu_ops(void) {}
> -static inline void *hv_alloc_hyperv_page(void) { return NULL; }
> -static inline void hv_free_hyperv_page(unsigned long addr) {}
>  static inline void set_hv_tscchange_cb(void (*cb)(void)) {}
>  static inline void clear_hv_tscchange_cb(void) {}
>  static inline void hyperv_stop_tsc_emulation(void) {};
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index f202ac7..cca8d5e 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -37,6 +37,42 @@ int hv_init(void)
>  }
>  
>  /*
> + * Functions for allocating and freeing memory with size and
> + * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> + * the guest page size may not be the same as the Hyper-V page
> + * size. We depend upon kmalloc() aligning power-of-two size
> + * allocations to the allocation size boundary, so that the
> + * allocated memory appears to Hyper-V as a page of the size
> + * it expects.
> + */
> +
> +void *hv_alloc_hyperv_page(void)
> +{
> + BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> +
> + if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> + return (void *)__get_free_page(GFP_KERNEL);
> + else
> + return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);

PAGE_SIZE and HV_HYP_PAGE_SIZE are known compile-time and in case this
won't change in the future we can probably write this as

#if PAGE_SIZE == HV_HYP_PAGE_SIZE
   return (void *)__get_free_page(GFP_KERNEL);
#else
   return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
#endif

(not sure if the output is going to be any different with e.g. gcc's '-O2')

> +}
> +
> +void *hv_alloc_hyperv_zeroed_page(void)
> +{
> + if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> + return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> + else
> + return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> +}
> +
> +void hv_free_hyperv_page(unsigned long addr)
> +{
> + if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> + free_page(addr);
> + else
> + kfree((void *)addr);
> +}
> +
> +/*
>   * hv_post_message - Post a message using the hypervisor message IPC.
>   *
>   * This involves a hypercall.
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index dff58a3..694b5bc 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -117,6 +117,10 @@ static inline void vmbus_signal_eom(struct hv_message 
> *msg, u32 old_msg_type)
>  /* Sentinel value for an uninitialized entry in hv_vp_index array */
>  #define VP_INVAL U32_MAX
>  
> +void *hv_alloc_hyperv_page(void);
> +void *hv_alloc_hyperv_zeroed_page(void);
> +void hv_free_hyperv_page(unsigned long addr);
> 

Alert-02

[Trustees]

We have been trying to reach you as regards the estate of Late George Brumley, 
you were made one of the beneficiaries of his estate. Do get back to me at your 
earliest convenience. The Trustees

[PATCH 5.9 230/391] btrfs: tree-checker: fix false alert caused by legacy btrfs root item

[Greg Kroah-Hartman]

From: Qu Wenruo 

commit 1465af12e254a68706e110846f59cf0f09683184 upstream.

Commit 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
introduced btrfs root item size check, however btrfs root item has two
versions, the legacy one which just ends before generation_v2 member, is
smaller than current btrfs root item size.

This caused btrfs kernel to reject valid but old tree root leaves.

Fix this problem by also allowing legacy root item, since kernel can
already handle them pretty well and upgrade to newer root item format
when needed.

Reported-by: Martin Steigerwald 
Fixes: 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
CC: sta...@vger.kernel.org # 5.4+
Tested-By: Martin Steigerwald 
Reviewed-by: Josef Bacik 
Signed-off-by: Qu Wenruo 
Reviewed-by: David Sterba 
Signed-off-by: David Sterba 
Signed-off-by: Greg Kroah-Hartman 

---
 fs/btrfs/tree-checker.c |   17 -
 include/uapi/linux/btrfs_tree.h |   14 ++
 2 files changed, 26 insertions(+), 5 deletions(-)

--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1035,7 +1035,7 @@ static int check_root_item(struct extent
   int slot)
 {
struct btrfs_fs_info *fs_info = leaf->fs_info;
-   struct btrfs_root_item ri;
+   struct btrfs_root_item ri = { 0 };
const u64 valid_root_flags = BTRFS_ROOT_SUBVOL_RDONLY |
 BTRFS_ROOT_SUBVOL_DEAD;
int ret;
@@ -1044,14 +1044,21 @@ static int check_root_item(struct extent
if (ret < 0)
return ret;
 
-   if (btrfs_item_size_nr(leaf, slot) != sizeof(ri)) {
+   if (btrfs_item_size_nr(leaf, slot) != sizeof(ri) &&
+   btrfs_item_size_nr(leaf, slot) != btrfs_legacy_root_item_size()) {
generic_err(leaf, slot,
-   "invalid root item size, have %u expect %zu",
-   btrfs_item_size_nr(leaf, slot), sizeof(ri));
+   "invalid root item size, have %u expect %zu or %u",
+   btrfs_item_size_nr(leaf, slot), sizeof(ri),
+   btrfs_legacy_root_item_size());
}
 
+   /*
+* For legacy root item, the members starting at generation_v2 will be
+* all filled with 0.
+* And since we allow geneartion_v2 as 0, it will still pass the check.
+*/
read_extent_buffer(leaf, , btrfs_item_ptr_offset(leaf, slot),
-  sizeof(ri));
+  btrfs_item_size_nr(leaf, slot));
 
/* Generation related */
if (btrfs_root_generation() >
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -4,6 +4,11 @@
 
 #include 
 #include 
+#ifdef __KERNEL__
+#include 
+#else
+#include 
+#endif
 
 /*
  * This header contains the structure definitions and constants used
@@ -645,6 +650,15 @@ struct btrfs_root_item {
 } __attribute__ ((__packed__));
 
 /*
+ * Btrfs root item used to be smaller than current size.  The old format ends
+ * at where member generation_v2 is.
+ */
+static inline __u32 btrfs_legacy_root_item_size(void)
+{
+   return offsetof(struct btrfs_root_item, generation_v2);
+}
+
+/*
  * this is used for both forward and backward root refs
  */
 struct btrfs_root_ref {

[PATCH 5.4 128/214] btrfs: tree-checker: fix false alert caused by legacy btrfs root item

[Greg Kroah-Hartman]

From: Qu Wenruo 

commit 1465af12e254a68706e110846f59cf0f09683184 upstream.

Commit 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
introduced btrfs root item size check, however btrfs root item has two
versions, the legacy one which just ends before generation_v2 member, is
smaller than current btrfs root item size.

This caused btrfs kernel to reject valid but old tree root leaves.

Fix this problem by also allowing legacy root item, since kernel can
already handle them pretty well and upgrade to newer root item format
when needed.

Reported-by: Martin Steigerwald 
Fixes: 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
CC: sta...@vger.kernel.org # 5.4+
Tested-By: Martin Steigerwald 
Reviewed-by: Josef Bacik 
Signed-off-by: Qu Wenruo 
Reviewed-by: David Sterba 
Signed-off-by: David Sterba 
Signed-off-by: Greg Kroah-Hartman 

---
 fs/btrfs/tree-checker.c |   17 -
 include/uapi/linux/btrfs_tree.h |   14 ++
 2 files changed, 26 insertions(+), 5 deletions(-)

--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -869,7 +869,7 @@ static int check_root_item(struct extent
   int slot)
 {
struct btrfs_fs_info *fs_info = leaf->fs_info;
-   struct btrfs_root_item ri;
+   struct btrfs_root_item ri = { 0 };
const u64 valid_root_flags = BTRFS_ROOT_SUBVOL_RDONLY |
 BTRFS_ROOT_SUBVOL_DEAD;
 
@@ -889,14 +889,21 @@ static int check_root_item(struct extent
return -EUCLEAN;
}
 
-   if (btrfs_item_size_nr(leaf, slot) != sizeof(ri)) {
+   if (btrfs_item_size_nr(leaf, slot) != sizeof(ri) &&
+   btrfs_item_size_nr(leaf, slot) != btrfs_legacy_root_item_size()) {
generic_err(leaf, slot,
-   "invalid root item size, have %u expect %zu",
-   btrfs_item_size_nr(leaf, slot), sizeof(ri));
+   "invalid root item size, have %u expect %zu or %u",
+   btrfs_item_size_nr(leaf, slot), sizeof(ri),
+   btrfs_legacy_root_item_size());
}
 
+   /*
+* For legacy root item, the members starting at generation_v2 will be
+* all filled with 0.
+* And since we allow geneartion_v2 as 0, it will still pass the check.
+*/
read_extent_buffer(leaf, , btrfs_item_ptr_offset(leaf, slot),
-  sizeof(ri));
+  btrfs_item_size_nr(leaf, slot));
 
/* Generation related */
if (btrfs_root_generation() >
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -4,6 +4,11 @@
 
 #include 
 #include 
+#ifdef __KERNEL__
+#include 
+#else
+#include 
+#endif
 
 /*
  * This header contains the structure definitions and constants used
@@ -651,6 +656,15 @@ struct btrfs_root_item {
 } __attribute__ ((__packed__));
 
 /*
+ * Btrfs root item used to be smaller than current size.  The old format ends
+ * at where member generation_v2 is.
+ */
+static inline __u32 btrfs_legacy_root_item_size(void)
+{
+   return offsetof(struct btrfs_root_item, generation_v2);
+}
+
+/*
  * this is used for both forward and backward root refs
  */
 struct btrfs_root_ref {

[PATCH v5 7/7] power: supply: max17040: Support soc alert

[Iskren Chernev]

max17048 and max17049 support SOC alerts (interrupts when battery
capacity changes by +/- 1%). At the moment the driver polls for changes
every second. Using the alerts removes the need for polling.

Signed-off-by: Iskren Chernev 
Tested-by: Jonathan Bakker 
---
 drivers/power/supply/max17040_battery.c | 82 ++---
 1 file changed, 73 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index ae39ca5c6753e..1d7510a59295d 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -25,6 +25,7 @@
 #define MAX17040_MODE  0x06
 #define MAX17040_VER   0x08
 #define MAX17040_CONFIG0x0C
+#define MAX17040_STATUS0x1A
 #define MAX17040_CMD   0xFE
 
 
@@ -33,7 +34,10 @@
 #define MAX17040_RCOMP_DEFAULT  0x9700
 
 #define MAX17040_ATHD_MASK 0x3f
+#define MAX17040_ALSC_MASK 0x40
 #define MAX17040_ATHD_DEFAULT_POWER_UP 4
+#define MAX17040_STATUS_HD_MASK0x1000
+#define MAX17040_STATUS_SC_MASK0x2000
 #define MAX17040_CFG_RCOMP_MASK0xff00
 
 enum chip_id {
@@ -55,6 +59,7 @@ struct chip_data {
u16 vcell_div;
u8  has_low_soc_alert;
u8  rcomp_bytes;
+   u8  has_soc_alert;
 };
 
 static struct chip_data max17040_family[] = {
@@ -65,6 +70,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 0,
.rcomp_bytes = 2,
+   .has_soc_alert = 0,
},
[ID_MAX17041] = {
.reset_val = 0x0054,
@@ -73,6 +79,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 0,
.rcomp_bytes = 2,
+   .has_soc_alert = 0,
},
[ID_MAX17043] = {
.reset_val = 0x0054,
@@ -81,6 +88,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17044] = {
.reset_val = 0x0054,
@@ -89,6 +97,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17048] = {
.reset_val = 0x5400,
@@ -97,6 +106,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 8,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 1,
},
[ID_MAX17049] = {
.reset_val = 0x5400,
@@ -105,6 +115,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 4,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 1,
},
[ID_MAX17058] = {
.reset_val = 0x5400,
@@ -113,6 +124,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 8,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17059] = {
.reset_val = 0x5400,
@@ -121,6 +133,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 4,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
 };
 
@@ -156,6 +169,12 @@ static int max17040_set_low_soc_alert(struct max17040_chip 
*chip, u32 level)
MAX17040_ATHD_MASK, level);
 }
 
+static int max17040_set_soc_alert(struct max17040_chip *chip, bool enable)
+{
+   return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+   MAX17040_ALSC_MASK, enable ? MAX17040_ALSC_MASK : 0);
+}
+
 static int max17040_set_rcomp(struct max17040_chip *chip, u16 rcomp)
 {
u16 mask = chip->data.rcomp_bytes == 2 ?
@@ -300,11 +319,33 @@ static void max17040_work(struct work_struct *work)
max17040_queue_work(chip);
 }
 
+/* Returns true if alert cause was SOC change, not low SOC */
+static bool max17040_handle_soc_alert(struct max17040_chip *chip)
+{
+   bool ret = true;
+   u32 data;
+
+   regmap_read(chip->regmap, MAX17040_STATUS, );
+
+   if (data & MAX17040_STATUS_HD_MASK) {
+   // this alert was caused by low soc
+   ret = false;
+   }
+   if (data & MAX17040_STATUS_SC_MASK) {
+   // soc change bit -- deassert to mark as handled
+   regmap_write(chip->regmap, MAX17040_STATUS,
+   data & ~MAX17040_STATUS_SC_MASK);
+   }
+
+   return ret;
+}
+
 static irqreturn_t max17040_thread_handler(int id, void *dev)
 {
struct max17040_chip *chip = dev;
 
-   dev_warn(>client->dev,

Re: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH 1/1] Drivers: hv: vmbus: Add timeout to vmbus_wait_for_unload

[Vitaly Kuznetsov]

Michael Kelley  writes:

> vmbus_wait_for_unload() looks for a CHANNELMSG_UNLOAD_RESPONSE message
> coming from Hyper-V.  But if the message isn't found for some reason,
> the panic path gets hung forever.  Add a timeout of 10 seconds to prevent
> this.

If I remember correctly, the problem I was observing back then was that
if CHANNELMSG_UNLOAD_RESPONSE is not delivered, Hyper-V won't respond to
the consequent CHANNELMSG_INITIATE_CONTACT/CHANNELMSG_REQUESTOFFERS
(don't remember exactly) so we either hang here or crash in the kdump
kernel because we can't find any devices. Maybe the problem was only
with some ancient Hyper-V versions or it was fixed.

>
> Fixes: 415719160de3 ("Drivers: hv: vmbus: avoid scheduling in interrupt 
> context in vmbus_initiate_unload()")
> Signed-off-by: Michael Kelley 
> ---
>  drivers/hv/channel_mgmt.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 591106c..1d44bb6 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -731,7 +731,7 @@ static void vmbus_wait_for_unload(void)
>   void *page_addr;
>   struct hv_message *msg;
>   struct vmbus_channel_message_header *hdr;
> - u32 message_type;
> + u32 message_type, i;
>  
>   /*
>* CHANNELMSG_UNLOAD_RESPONSE is always delivered to the CPU which was
> @@ -741,8 +741,11 @@ static void vmbus_wait_for_unload(void)
>* functional and vmbus_unload_response() will complete
>* vmbus_connection.unload_event. If not, the last thing we can do is
>* read message pages for all CPUs directly.
> +  *
> +  * Wait no more than 10 seconds so that the panic path can't get
> +  * hung forever in case the response message isn't seen.
>*/
> - while (1) {
> + for (i = 0; i < 1000; i++) {
>   if (completion_done(_connection.unload_event))
>   break;

LGTM,

Reviewed-by: Vitaly Kuznetsov 

-- 
Vitaly

Re: [PATCH] i2c: stm32f7: add SMBus-Alert support

[Pierre Yves MORDRET]

Hi Alain

Sounds good

Reviewed-by: Pierre-Yves MORDRET 

Best Regards


On 8/3/20 7:26 AM, Alain Volmat wrote:
> Add support for the SMBus-Alert protocol.
> 
> Signed-off-by: Alain Volmat 
> ---
>  This patch has to be integrated on top of the patch
>  'i2c: stm32f7: Add SMBus Host-Notify protocol support' since SMBus Alert is
>  enabled by the DT binding 'smbus' introduced in that patch.
> 
>  drivers/i2c/busses/i2c-stm32f7.c | 71 
> 
>  1 file changed, 71 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c 
> b/drivers/i2c/busses/i2c-stm32f7.c
> index 223c238c3c09..fe7641da54ef 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -51,6 +51,7 @@
>  
>  /* STM32F7 I2C control 1 */
>  #define STM32F7_I2C_CR1_PECENBIT(23)
> +#define STM32F7_I2C_CR1_ALERTEN  BIT(22)
>  #define STM32F7_I2C_CR1_SMBHEN   BIT(20)
>  #define STM32F7_I2C_CR1_WUPENBIT(18)
>  #define STM32F7_I2C_CR1_SBC  BIT(16)
> @@ -123,6 +124,7 @@
>   (((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
>  #define STM32F7_I2C_ISR_DIR  BIT(16)
>  #define STM32F7_I2C_ISR_BUSY BIT(15)
> +#define STM32F7_I2C_ISR_ALERTBIT(13)
>  #define STM32F7_I2C_ISR_PECERR   BIT(11)
>  #define STM32F7_I2C_ISR_ARLO BIT(9)
>  #define STM32F7_I2C_ISR_BERR BIT(8)
> @@ -136,6 +138,7 @@
>  #define STM32F7_I2C_ISR_TXE  BIT(0)
>  
>  /* STM32F7 I2C Interrupt Clear */
> +#define STM32F7_I2C_ICR_ALERTCF  BIT(13)
>  #define STM32F7_I2C_ICR_PECCFBIT(11)
>  #define STM32F7_I2C_ICR_ARLOCF   BIT(9)
>  #define STM32F7_I2C_ICR_BERRCF   BIT(8)
> @@ -277,6 +280,17 @@ struct stm32f7_i2c_msg {
>  };
>  
>  /**
> + * struct stm32f7_i2c_alert - SMBus alert specific data
> + * @setup: platform data for the smbus_alert i2c client
> + * @ara: I2C slave device used to respond to the SMBus Alert with Alert
> + * Response Address
> + */
> +struct stm32f7_i2c_alert {
> + struct i2c_smbus_alert_setup setup;
> + struct i2c_client *ara;
> +};
> +
> +/**
>   * struct stm32f7_i2c_dev - private data of the controller
>   * @adap: I2C adapter for this controller
>   * @dev: device for this controller
> @@ -305,6 +319,7 @@ struct stm32f7_i2c_msg {
>   * @wakeup_src: boolean to know if the device is a wakeup source
>   * @smbus_mode: states that the controller is configured in SMBus mode
>   * @host_notify_client: SMBus host-notify client
> + * @alert: SMBus alert specific data
>   */
>  struct stm32f7_i2c_dev {
>   struct i2c_adapter adap;
> @@ -333,6 +348,7 @@ struct stm32f7_i2c_dev {
>   bool wakeup_src;
>   bool smbus_mode;
>   struct i2c_client *host_notify_client;
> + struct stm32f7_i2c_alert *alert;
>  };
>  
>  /*
> @@ -1601,6 +1617,13 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
> *data)
>   f7_msg->result = -EINVAL;
>   }
>  
> + if (status & STM32F7_I2C_ISR_ALERT) {
> + dev_dbg(dev, "<%s>: SMBus alert received\n", __func__);
> + writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
> + i2c_handle_smbus_alert(i2c_dev->alert->ara);
> + return IRQ_HANDLED;
> + }
> +
>   if (!i2c_dev->slave_running) {
>   u32 mask;
>   /* Disable interrupts */
> @@ -1967,6 +1990,42 @@ static void stm32f7_i2c_disable_smbus_host(struct 
> stm32f7_i2c_dev *i2c_dev)
>   }
>  }
>  
> +static int stm32f7_i2c_enable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
> +{
> + struct stm32f7_i2c_alert *alert;
> +     struct i2c_adapter *adap = _dev->adap;
> + struct device *dev = i2c_dev->dev;
> + void __iomem *base = i2c_dev->base;
> +
> + alert = devm_kzalloc(dev, sizeof(*alert), GFP_KERNEL);
> + if (!alert)
> + return -ENOMEM;
> +
> + alert->ara = i2c_new_smbus_alert_device(adap, >setup);
> + if (IS_ERR(alert->ara))
> + return PTR_ERR(alert->ara);
> +
> + i2c_dev->alert = alert;
> +
> + /* Enable SMBus Alert */
> + stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, STM32F7_I2C_CR1_ALERTEN);
> +
> + return 0;
> +}
> +
> +static void stm32f7_i2c_disable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
> +{
> + struct stm32f7_i2c_alert *alert = i2c_dev->alert

[PATCH v4 7/7] power: supply: max17040: Support soc alert

[Iskren Chernev]

max17048 and max17049 support SOC alerts (interrupts when battery
capacity changes by +/- 1%). At the moment the driver polls for changes
every second. Using the alerts removes the need for polling.

Signed-off-by: Iskren Chernev 
Tested-by: Jonathan Bakker 
---
 drivers/power/supply/max17040_battery.c | 82 ++---
 1 file changed, 73 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 6ead2fded6a96..1c4cb7ddc785c 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -25,6 +25,7 @@
 #define MAX17040_MODE  0x06
 #define MAX17040_VER   0x08
 #define MAX17040_CONFIG0x0C
+#define MAX17040_STATUS0x1A
 #define MAX17040_CMD   0xFE
 
 
@@ -33,7 +34,10 @@
 #define MAX17040_RCOMP_DEFAULT  0x9700
 
 #define MAX17040_ATHD_MASK 0x3f
+#define MAX17040_ALSC_MASK 0x40
 #define MAX17040_ATHD_DEFAULT_POWER_UP 4
+#define MAX17040_STATUS_HD_MASK0x1000
+#define MAX17040_STATUS_SC_MASK0x2000
 #define MAX17040_CFG_RCOMP_MASK0xff00
 
 enum chip_id {
@@ -55,6 +59,7 @@ struct chip_data {
u16 vcell_div;
u8  has_low_soc_alert;
u8  rcomp_bytes;
+   u8  has_soc_alert;
 };
 
 static struct chip_data max17040_family[] = {
@@ -65,6 +70,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 0,
.rcomp_bytes = 2,
+   .has_soc_alert = 0,
},
[ID_MAX17041] = {
.reset_val = 0x0054,
@@ -73,6 +79,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 0,
.rcomp_bytes = 2,
+   .has_soc_alert = 0,
},
[ID_MAX17043] = {
.reset_val = 0x0054,
@@ -81,6 +88,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17044] = {
.reset_val = 0x0054,
@@ -89,6 +97,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17048] = {
.reset_val = 0x5400,
@@ -97,6 +106,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 8,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 1,
},
[ID_MAX17049] = {
.reset_val = 0x5400,
@@ -105,6 +115,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 4,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 1,
},
[ID_MAX17058] = {
.reset_val = 0x5400,
@@ -113,6 +124,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 8,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17059] = {
.reset_val = 0x5400,
@@ -121,6 +133,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 4,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
 };
 
@@ -156,6 +169,12 @@ static int max17040_set_low_soc_alert(struct max17040_chip 
*chip, u32 level)
MAX17040_ATHD_MASK, level);
 }
 
+static int max17040_set_soc_alert(struct max17040_chip *chip, bool enable)
+{
+   return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+   MAX17040_ALSC_MASK, enable ? MAX17040_ALSC_MASK : 0);
+}
+
 static int max17040_set_rcomp(struct max17040_chip *chip, u16 rcomp)
 {
u16 mask = chip->data.rcomp_bytes == 2 ?
@@ -300,11 +319,33 @@ static void max17040_work(struct work_struct *work)
max17040_queue_work(chip);
 }
 
+/* Returns true if alert cause was SOC change, not low SOC */
+static bool max17040_handle_soc_alert(struct max17040_chip *chip)
+{
+   bool ret = true;
+   u32 data;
+
+   regmap_read(chip->regmap, MAX17040_STATUS, );
+
+   if (data & MAX17040_STATUS_HD_MASK) {
+   // this alert was caused by low soc
+   ret = false;
+   }
+   if (data & MAX17040_STATUS_SC_MASK) {
+   // soc change bit -- deassert to mark as handled
+   regmap_write(chip->regmap, MAX17040_STATUS,
+   data & ~MAX17040_STATUS_SC_MASK);
+   }
+
+   return ret;
+}
+
 static irqreturn_t max17040_thread_handler(int id, void *dev)
 {
struct max17040_chip *chip = dev;
 
-   dev_warn(>client->dev,

[PATCH v3 14/19] dlb2: add domain alert support

[Gage Eads]

Domain alerts are a mechanism for the driver to asynchronously notify
user-space applications of device reset or hardware alarms (both to be
added in later commits). This mechanism also allows the application to
enqueue an alert to its domain, as a form of (limited) IPC in a
multi-process scenario.

An application can read its domain alerts through the domain device file's
read callback. Applications are expected to spawn a thread that performs a
blocking read, and rarely (if ever) wakes and returns to user-space.

Signed-off-by: Gage Eads 
Reviewed-by: Björn Töpel 
---
 drivers/misc/dlb2/dlb2_ioctl.c |  17 ++
 drivers/misc/dlb2/dlb2_main.c  | 130 +++
 drivers/misc/dlb2/dlb2_main.h  |  16 +
 include/uapi/linux/dlb2_user.h | 134 +
 4 files changed, 297 insertions(+)

diff --git a/drivers/misc/dlb2/dlb2_ioctl.c b/drivers/misc/dlb2/dlb2_ioctl.c
index 010e67941cf9..2350d8ff823e 100644
--- a/drivers/misc/dlb2/dlb2_ioctl.c
+++ b/drivers/misc/dlb2/dlb2_ioctl.c
@@ -493,6 +493,21 @@ static int dlb2_domain_ioctl_get_dir_port_cq_fd(struct 
dlb2_dev *dev,
   "dlb2_dir_cq:", _cq_fops, false);
 }
 
+static int dlb2_domain_ioctl_enqueue_domain_alert(struct dlb2_dev *dev,
+ struct dlb2_domain *domain,
+ unsigned long user_arg)
+{
+   struct dlb2_enqueue_domain_alert_args arg;
+
+   if (copy_from_user(, (void __user *)user_arg, sizeof(arg)))
+   return -EFAULT;
+
+   return dlb2_write_domain_alert(dev,
+  domain,
+  DLB2_DOMAIN_ALERT_USER,
+  arg.aux_alert_data);
+}
+
 long dlb2_domain_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 {
struct dlb2_domain *dom = f->private_data;
@@ -537,6 +552,8 @@ long dlb2_domain_ioctl(struct file *f, unsigned int cmd, 
unsigned long arg)
return dlb2_domain_ioctl_disable_dir_port(dev, dom, arg);
case DLB2_IOC_BLOCK_ON_CQ_INTERRUPT:
return dlb2_domain_ioctl_block_on_cq_interrupt(dev, dom, arg);
+   case DLB2_IOC_ENQUEUE_DOMAIN_ALERT:
+   return dlb2_domain_ioctl_enqueue_domain_alert(dev, dom, arg);
default:
return -ENOTTY;
}
diff --git a/drivers/misc/dlb2/dlb2_main.c b/drivers/misc/dlb2/dlb2_main.c
index b542c2c081a5..b457bda7be44 100644
--- a/drivers/misc/dlb2/dlb2_main.c
+++ b/drivers/misc/dlb2/dlb2_main.c
@@ -101,6 +101,9 @@ int dlb2_init_domain(struct dlb2_dev *dlb2_dev, u32 
domain_id)
kref_init(>refcnt);
domain->dlb2_dev = dlb2_dev;
 
+   spin_lock_init(>alert_lock);
+   init_waitqueue_head(>wq_head);
+
dlb2_dev->sched_domains[domain_id] = domain;
 
dlb2_dev->ops->inc_pm_refcnt(dlb2_dev->pdev, true);
@@ -227,9 +230,136 @@ static int dlb2_domain_close(struct inode *i, struct file 
*f)
return ret;
 }
 
+int dlb2_write_domain_alert(struct dlb2_dev *dev,
+   struct dlb2_domain *domain,
+   u64 alert_id,
+   u64 aux_alert_data)
+{
+   struct dlb2_domain_alert alert;
+   int idx;
+
+   if (!domain)
+   return -EINVAL;
+
+   /* Grab the alert lock to access the read and write indexes */
+   spin_lock(>alert_lock);
+
+   /* If there's no space for this notification, return */
+   if ((domain->alert_wr_idx - domain->alert_rd_idx) ==
+   (DLB2_DOMAIN_ALERT_RING_SIZE - 1)) {
+   spin_unlock(>alert_lock);
+   return 0;
+   }
+
+   alert.alert_id = alert_id;
+   alert.aux_alert_data = aux_alert_data;
+
+   idx = domain->alert_wr_idx % DLB2_DOMAIN_ALERT_RING_SIZE;
+
+   domain->alerts[idx] = alert;
+
+   domain->alert_wr_idx++;
+
+   spin_unlock(>alert_lock);
+
+   /* Wake any blocked readers */
+   wake_up_interruptible(>wq_head);
+
+   return 0;
+}
+
+static bool dlb2_alerts_avail(struct dlb2_domain *domain)
+{
+   bool ret;
+
+   spin_lock(>alert_lock);
+
+   ret = domain->alert_rd_idx != domain->alert_wr_idx;
+
+   spin_unlock(>alert_lock);
+
+   return ret;
+}
+
+static int dlb2_read_domain_alert(struct dlb2_dev *dev,
+ struct dlb2_domain *domain,
+ struct dlb2_domain_alert *alert,
+     bool nonblock)
+{
+   int idx;
+
+   /* Grab the alert lock to access the read and write indexes */
+   spin_lock(>alert_lock);
+
+   while (domain->alert_rd_idx == domain->alert_wr_idx) {
+   /*
+* Release the alert lock before putting the thread on the wait
+* queue.
+  

[PATCH 5.4 188/214] drm/amd/pm: correct the thermal alert temperature limit settings

[Greg Kroah-Hartman]

From: Evan Quan 

commit 28e628645333b7e581c4a7b04d958e4804ea10fe upstream.

Do the maths in celsius degree. This can fix the issues caused
by the changes below:

drm/amd/pm: correct Vega20 swctf limit setting
drm/amd/pm: correct Vega12 swctf limit setting
drm/amd/pm: correct Vega10 swctf limit setting

Signed-off-by: Evan Quan 
Reviewed-by: Kenneth Feng 
Signed-off-by: Alex Deucher 
Cc: sta...@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c |   15 +++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_thermal.c |   15 +++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_thermal.c |   15 +++
 3 files changed, 21 insertions(+), 24 deletions(-)

--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
@@ -368,14 +368,13 @@ static int vega10_thermal_set_temperatur
(struct phm_ppt_v2_information *)(hwmgr->pptable);
struct phm_tdp_table *tdp_table = pp_table_info->tdp_table;
struct amdgpu_device *adev = hwmgr->adev;
-   int low = VEGA10_THERMAL_MINIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
-   int high = VEGA10_THERMAL_MAXIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
+   int low = VEGA10_THERMAL_MINIMUM_ALERT_TEMP;
+   int high = VEGA10_THERMAL_MAXIMUM_ALERT_TEMP;
uint32_t val;
 
-   if (low < range->min)
-   low = range->min;
+   /* compare them in unit celsius degree */
+   if (low < range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES)
+   low = range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
if (high > tdp_table->usSoftwareShutdownTemp)
high = tdp_table->usSoftwareShutdownTemp;
 
@@ -386,8 +385,8 @@ static int vega10_thermal_set_temperatur
 
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, MAX_IH_CREDIT, 5);
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, THERM_IH_HW_ENA, 1);
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, (high / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, (low / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, high);
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, low);
val &= (~THM_THERMAL_INT_CTRL__THERM_TRIGGER_MASK_MASK) &
(~THM_THERMAL_INT_CTRL__THERM_INTH_MASK_MASK) &
(~THM_THERMAL_INT_CTRL__THERM_INTL_MASK_MASK);
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_thermal.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_thermal.c
@@ -173,14 +173,13 @@ static int vega12_thermal_set_temperatur
struct phm_ppt_v3_information *pptable_information =
(struct phm_ppt_v3_information *)hwmgr->pptable;
struct amdgpu_device *adev = hwmgr->adev;
-   int low = VEGA12_THERMAL_MINIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
-   int high = VEGA12_THERMAL_MAXIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
+   int low = VEGA12_THERMAL_MINIMUM_ALERT_TEMP;
+   int high = VEGA12_THERMAL_MAXIMUM_ALERT_TEMP;
uint32_t val;
 
-   if (low < range->min)
-   low = range->min;
+   /* compare them in unit celsius degree */
+   if (low < range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES)
+   low = range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
if (high > pptable_information->us_software_shutdown_temp)
high = pptable_information->us_software_shutdown_temp;
 
@@ -191,8 +190,8 @@ static int vega12_thermal_set_temperatur
 
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, MAX_IH_CREDIT, 5);
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, THERM_IH_HW_ENA, 1);
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, (high / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, (low / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, high);
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, low);
val = val & (~THM_THERMAL_INT_CTRL__THERM_TRIGGER_MASK_MASK);
 
WREG32_SOC15(THM, 0, mmTHM_THERMAL_INT_CTRL, val);
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_thermal.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_thermal.c
@@ -243,14 +243,13 @@ static int vega20_thermal_set_temperatur
struct phm_ppt_v3_information *pptable_information =
(struct phm_ppt_v3_information *)hwmgr->pptable;
struct amdgpu_device *adev = hwmgr->adev;
-   int low = VEGA20_THERMAL_MINIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
-   int high 

[PATCH 5.8 221/255] drm/amd/pm: correct the thermal alert temperature limit settings

[Greg Kroah-Hartman]

From: Evan Quan 

commit 28e628645333b7e581c4a7b04d958e4804ea10fe upstream.

Do the maths in celsius degree. This can fix the issues caused
by the changes below:

drm/amd/pm: correct Vega20 swctf limit setting
drm/amd/pm: correct Vega12 swctf limit setting
drm/amd/pm: correct Vega10 swctf limit setting

Signed-off-by: Evan Quan 
Reviewed-by: Kenneth Feng 
Signed-off-by: Alex Deucher 
Cc: sta...@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c |   15 +++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_thermal.c |   15 +++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_thermal.c |   15 +++
 3 files changed, 21 insertions(+), 24 deletions(-)

--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
@@ -367,14 +367,13 @@ static int vega10_thermal_set_temperatur
(struct phm_ppt_v2_information *)(hwmgr->pptable);
struct phm_tdp_table *tdp_table = pp_table_info->tdp_table;
struct amdgpu_device *adev = hwmgr->adev;
-   int low = VEGA10_THERMAL_MINIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
-   int high = VEGA10_THERMAL_MAXIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
+   int low = VEGA10_THERMAL_MINIMUM_ALERT_TEMP;
+   int high = VEGA10_THERMAL_MAXIMUM_ALERT_TEMP;
uint32_t val;
 
-   if (low < range->min)
-   low = range->min;
+   /* compare them in unit celsius degree */
+   if (low < range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES)
+   low = range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
if (high > tdp_table->usSoftwareShutdownTemp)
high = tdp_table->usSoftwareShutdownTemp;
 
@@ -385,8 +384,8 @@ static int vega10_thermal_set_temperatur
 
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, MAX_IH_CREDIT, 5);
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, THERM_IH_HW_ENA, 1);
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, (high / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, (low / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, high);
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, low);
val &= (~THM_THERMAL_INT_CTRL__THERM_TRIGGER_MASK_MASK) &
(~THM_THERMAL_INT_CTRL__THERM_INTH_MASK_MASK) &
(~THM_THERMAL_INT_CTRL__THERM_INTL_MASK_MASK);
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_thermal.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_thermal.c
@@ -173,14 +173,13 @@ static int vega12_thermal_set_temperatur
struct phm_ppt_v3_information *pptable_information =
(struct phm_ppt_v3_information *)hwmgr->pptable;
struct amdgpu_device *adev = hwmgr->adev;
-   int low = VEGA12_THERMAL_MINIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
-   int high = VEGA12_THERMAL_MAXIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
+   int low = VEGA12_THERMAL_MINIMUM_ALERT_TEMP;
+   int high = VEGA12_THERMAL_MAXIMUM_ALERT_TEMP;
uint32_t val;
 
-   if (low < range->min)
-   low = range->min;
+   /* compare them in unit celsius degree */
+   if (low < range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES)
+   low = range->min / PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
if (high > pptable_information->us_software_shutdown_temp)
high = pptable_information->us_software_shutdown_temp;
 
@@ -191,8 +190,8 @@ static int vega12_thermal_set_temperatur
 
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, MAX_IH_CREDIT, 5);
val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, THERM_IH_HW_ENA, 1);
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, (high / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
-   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, (low / 
PP_TEMPERATURE_UNITS_PER_CENTIGRADES));
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTH, high);
+   val = REG_SET_FIELD(val, THM_THERMAL_INT_CTRL, DIG_THERM_INTL, low);
val = val & (~THM_THERMAL_INT_CTRL__THERM_TRIGGER_MASK_MASK);
 
WREG32_SOC15(THM, 0, mmTHM_THERMAL_INT_CTRL, val);
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_thermal.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_thermal.c
@@ -243,14 +243,13 @@ static int vega20_thermal_set_temperatur
struct phm_ppt_v3_information *pptable_information =
(struct phm_ppt_v3_information *)hwmgr->pptable;
struct amdgpu_device *adev = hwmgr->adev;
-   int low = VEGA20_THERMAL_MINIMUM_ALERT_TEMP *
-   PP_TEMPERATURE_UNITS_PER_CENTIGRADES;
-   int high 

[PATCH v2 14/19] dlb2: add domain alert support

[Gage Eads]

Domain alerts are a mechanism for the driver to asynchronously notify
user-space applications of device reset or hardware alarms (both to be
added in later commits). This mechanism also allows the application to
enqueue an alert to its domain, as a form of (limited) IPC in a
multi-process scenario.

An application can read its domain alerts through the domain device file's
read callback. Applications are expected to spawn a thread that performs a
blocking read, and rarely (if ever) wakes and returns to user-space.

Signed-off-by: Gage Eads 
Reviewed-by: Björn Töpel 
---
 drivers/misc/dlb2/dlb2_ioctl.c |  17 ++
 drivers/misc/dlb2/dlb2_main.c  | 130 +++
 drivers/misc/dlb2/dlb2_main.h  |  16 +
 include/uapi/linux/dlb2_user.h | 134 +
 4 files changed, 297 insertions(+)

diff --git a/drivers/misc/dlb2/dlb2_ioctl.c b/drivers/misc/dlb2/dlb2_ioctl.c
index 010e67941cf9..2350d8ff823e 100644
--- a/drivers/misc/dlb2/dlb2_ioctl.c
+++ b/drivers/misc/dlb2/dlb2_ioctl.c
@@ -493,6 +493,21 @@ static int dlb2_domain_ioctl_get_dir_port_cq_fd(struct 
dlb2_dev *dev,
   "dlb2_dir_cq:", _cq_fops, false);
 }
 
+static int dlb2_domain_ioctl_enqueue_domain_alert(struct dlb2_dev *dev,
+ struct dlb2_domain *domain,
+ unsigned long user_arg)
+{
+   struct dlb2_enqueue_domain_alert_args arg;
+
+   if (copy_from_user(, (void __user *)user_arg, sizeof(arg)))
+   return -EFAULT;
+
+   return dlb2_write_domain_alert(dev,
+  domain,
+  DLB2_DOMAIN_ALERT_USER,
+  arg.aux_alert_data);
+}
+
 long dlb2_domain_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 {
struct dlb2_domain *dom = f->private_data;
@@ -537,6 +552,8 @@ long dlb2_domain_ioctl(struct file *f, unsigned int cmd, 
unsigned long arg)
return dlb2_domain_ioctl_disable_dir_port(dev, dom, arg);
case DLB2_IOC_BLOCK_ON_CQ_INTERRUPT:
return dlb2_domain_ioctl_block_on_cq_interrupt(dev, dom, arg);
+   case DLB2_IOC_ENQUEUE_DOMAIN_ALERT:
+   return dlb2_domain_ioctl_enqueue_domain_alert(dev, dom, arg);
default:
return -ENOTTY;
}
diff --git a/drivers/misc/dlb2/dlb2_main.c b/drivers/misc/dlb2/dlb2_main.c
index b542c2c081a5..b457bda7be44 100644
--- a/drivers/misc/dlb2/dlb2_main.c
+++ b/drivers/misc/dlb2/dlb2_main.c
@@ -101,6 +101,9 @@ int dlb2_init_domain(struct dlb2_dev *dlb2_dev, u32 
domain_id)
kref_init(>refcnt);
domain->dlb2_dev = dlb2_dev;
 
+   spin_lock_init(>alert_lock);
+   init_waitqueue_head(>wq_head);
+
dlb2_dev->sched_domains[domain_id] = domain;
 
dlb2_dev->ops->inc_pm_refcnt(dlb2_dev->pdev, true);
@@ -227,9 +230,136 @@ static int dlb2_domain_close(struct inode *i, struct file 
*f)
return ret;
 }
 
+int dlb2_write_domain_alert(struct dlb2_dev *dev,
+   struct dlb2_domain *domain,
+   u64 alert_id,
+   u64 aux_alert_data)
+{
+   struct dlb2_domain_alert alert;
+   int idx;
+
+   if (!domain)
+   return -EINVAL;
+
+   /* Grab the alert lock to access the read and write indexes */
+   spin_lock(>alert_lock);
+
+   /* If there's no space for this notification, return */
+   if ((domain->alert_wr_idx - domain->alert_rd_idx) ==
+   (DLB2_DOMAIN_ALERT_RING_SIZE - 1)) {
+   spin_unlock(>alert_lock);
+   return 0;
+   }
+
+   alert.alert_id = alert_id;
+   alert.aux_alert_data = aux_alert_data;
+
+   idx = domain->alert_wr_idx % DLB2_DOMAIN_ALERT_RING_SIZE;
+
+   domain->alerts[idx] = alert;
+
+   domain->alert_wr_idx++;
+
+   spin_unlock(>alert_lock);
+
+   /* Wake any blocked readers */
+   wake_up_interruptible(>wq_head);
+
+   return 0;
+}
+
+static bool dlb2_alerts_avail(struct dlb2_domain *domain)
+{
+   bool ret;
+
+   spin_lock(>alert_lock);
+
+   ret = domain->alert_rd_idx != domain->alert_wr_idx;
+
+   spin_unlock(>alert_lock);
+
+   return ret;
+}
+
+static int dlb2_read_domain_alert(struct dlb2_dev *dev,
+ struct dlb2_domain *domain,
+ struct dlb2_domain_alert *alert,
+     bool nonblock)
+{
+   int idx;
+
+   /* Grab the alert lock to access the read and write indexes */
+   spin_lock(>alert_lock);
+
+   while (domain->alert_rd_idx == domain->alert_wr_idx) {
+   /*
+* Release the alert lock before putting the thread on the wait
+* queue.
+  

[PATCH] i2c: stm32f7: add SMBus-Alert support

[Alain Volmat]

Add support for the SMBus-Alert protocol.

Signed-off-by: Alain Volmat 
---
 This patch has to be integrated on top of the patch
 'i2c: stm32f7: Add SMBus Host-Notify protocol support' since SMBus Alert is
 enabled by the DT binding 'smbus' introduced in that patch.

 drivers/i2c/busses/i2c-stm32f7.c | 71 
 1 file changed, 71 insertions(+)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 223c238c3c09..fe7641da54ef 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -51,6 +51,7 @@
 
 /* STM32F7 I2C control 1 */
 #define STM32F7_I2C_CR1_PECEN  BIT(23)
+#define STM32F7_I2C_CR1_ALERTENBIT(22)
 #define STM32F7_I2C_CR1_SMBHEN BIT(20)
 #define STM32F7_I2C_CR1_WUPEN  BIT(18)
 #define STM32F7_I2C_CR1_SBCBIT(16)
@@ -123,6 +124,7 @@
(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
 #define STM32F7_I2C_ISR_DIRBIT(16)
 #define STM32F7_I2C_ISR_BUSY   BIT(15)
+#define STM32F7_I2C_ISR_ALERT  BIT(13)
 #define STM32F7_I2C_ISR_PECERR BIT(11)
 #define STM32F7_I2C_ISR_ARLO   BIT(9)
 #define STM32F7_I2C_ISR_BERR   BIT(8)
@@ -136,6 +138,7 @@
 #define STM32F7_I2C_ISR_TXEBIT(0)
 
 /* STM32F7 I2C Interrupt Clear */
+#define STM32F7_I2C_ICR_ALERTCFBIT(13)
 #define STM32F7_I2C_ICR_PECCF  BIT(11)
 #define STM32F7_I2C_ICR_ARLOCF BIT(9)
 #define STM32F7_I2C_ICR_BERRCF BIT(8)
@@ -277,6 +280,17 @@ struct stm32f7_i2c_msg {
 };
 
 /**
+ * struct stm32f7_i2c_alert - SMBus alert specific data
+ * @setup: platform data for the smbus_alert i2c client
+ * @ara: I2C slave device used to respond to the SMBus Alert with Alert
+ * Response Address
+ */
+struct stm32f7_i2c_alert {
+   struct i2c_smbus_alert_setup setup;
+   struct i2c_client *ara;
+};
+
+/**
  * struct stm32f7_i2c_dev - private data of the controller
  * @adap: I2C adapter for this controller
  * @dev: device for this controller
@@ -305,6 +319,7 @@ struct stm32f7_i2c_msg {
  * @wakeup_src: boolean to know if the device is a wakeup source
  * @smbus_mode: states that the controller is configured in SMBus mode
  * @host_notify_client: SMBus host-notify client
+ * @alert: SMBus alert specific data
  */
 struct stm32f7_i2c_dev {
struct i2c_adapter adap;
@@ -333,6 +348,7 @@ struct stm32f7_i2c_dev {
bool wakeup_src;
bool smbus_mode;
struct i2c_client *host_notify_client;
+   struct stm32f7_i2c_alert *alert;
 };
 
 /*
@@ -1601,6 +1617,13 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void 
*data)
f7_msg->result = -EINVAL;
}
 
+   if (status & STM32F7_I2C_ISR_ALERT) {
+   dev_dbg(dev, "<%s>: SMBus alert received\n", __func__);
+   writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
+   i2c_handle_smbus_alert(i2c_dev->alert->ara);
+   return IRQ_HANDLED;
+   }
+
if (!i2c_dev->slave_running) {
u32 mask;
/* Disable interrupts */
@@ -1967,6 +1990,42 @@ static void stm32f7_i2c_disable_smbus_host(struct 
stm32f7_i2c_dev *i2c_dev)
}
 }
 
+static int stm32f7_i2c_enable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
+{
+   struct stm32f7_i2c_alert *alert;
+   struct i2c_adapter *adap = _dev->adap;
+   struct device *dev = i2c_dev->dev;
+   void __iomem *base = i2c_dev->base;
+
+   alert = devm_kzalloc(dev, sizeof(*alert), GFP_KERNEL);
+   if (!alert)
+   return -ENOMEM;
+
+   alert->ara = i2c_new_smbus_alert_device(adap, >setup);
+   if (IS_ERR(alert->ara))
+       return PTR_ERR(alert->ara);
+
+   i2c_dev->alert = alert;
+
+   /* Enable SMBus Alert */
+   stm32f7_i2c_set_bits(base + STM32F7_I2C_CR1, STM32F7_I2C_CR1_ALERTEN);
+
+   return 0;
+}
+
+static void stm32f7_i2c_disable_smbus_alert(struct stm32f7_i2c_dev *i2c_dev)
+{
+   struct stm32f7_i2c_alert *alert = i2c_dev->alert;
+   void __iomem *base = i2c_dev->base;
+
+   if (alert) {
+   /* Disable SMBus Alert */
+   stm32f7_i2c_clr_bits(base + STM32F7_I2C_CR1,
+    STM32F7_I2C_CR1_ALERTEN);
+   i2c_unregister_device(alert->ara);
+   }
+}
+
 static u32 stm32f7_i2c_func(struct i2c_adapter *adap)
 {
struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
@@ -2161,6 +2220,14 @@ static int stm32f7_i2c_probe(struct platform_device 
*pdev)
ret);
goto i2c_adapter_remove;
}
+
+   ret = stm32f7_i2c_enable_smbus_alert(i2c_de

[PATCH 5.7 075/179] crypto/chtls: fix tls alert messages corrupted by tls data

[Greg Kroah-Hartman]

From: Vinay Kumar Yadav 

[ Upstream commit c271042eb6a031d1333cf57422ec1d20726901ab ]

When tls data skb is pending for Tx and tls alert comes , It
is wrongly overwrite the record type of tls data to tls alert
record type. fix the issue correcting it.

v1->v2:
- Correct submission tree.
- Add fixes tag.

Fixes: 6919a8264a32 ("Crypto/chtls: add/delete TLS header in driver")
Signed-off-by: Vinay Kumar Yadav 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 drivers/crypto/chelsio/chtls/chtls_io.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c 
b/drivers/crypto/chelsio/chtls/chtls_io.c
index e1401d9cc756c..2e9acae1cba3b 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -1052,14 +1052,15 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, 
size_t size)
  _type);
if (err)
goto out_err;
+
+   /* Avoid appending tls handshake, alert to tls 
data */
+   if (skb)
+   tx_skb_finalize(skb);
}
 
recordsz = size;
csk->tlshws.txleft = recordsz;
csk->tlshws.type = record_type;
-
-   if (skb)
-   ULP_SKB_CB(skb)->ulp.tls.type = record_type;
}
 
if (!skb || (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND) ||
-- 
2.25.1

Re: [PATCH v2 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Wolfram Sang]

Hi Rob,

> > > The I2C/SMBUS framework already provides a mechanism to enable SMBus-Alert
> > > by naming an IRQ line "smbus_alert". However, on stm32, the SMBus-Alert is
> > > part of the i2c IRQ. Using the smbus_alert naming here would lead to 
> > > having
> > > 2 handlers (the handler of the driver and the smbus_alert handler
> > > from I2C/SMBUS framework) on the unique i2c IRQ of the stm32. Meaning that
> > > the smbus_alert handler would get called for all IRQ generated by the 
> > > stm32
> > > I2C controller.
> > > 
> > > For that reason, the smbus_alert IRQ naming cannot be used and a dedicated
> > > binding is introduced.
> > 
> > What if we update the core to not register another irq handler if the
> > "smbus_alert" and main irq are the same?
> > 
> > I think it could work. However, while trying to make a proof-of-concept,
> > I found that irq descriptions in the generic i2c binding document are
> > probably mixed up. And before fixing that, I'd like to get HostNotify
> > done first.
> 
> Why does this even need to be in DT? Can't the driver just register that 
> it supports SMBus alert or have some call to the core signaling an SMBus 
> alert? 

If we emulate this SMBus behaviour with I2C, it means we apply
additional restrictions. In this case, there is an address which can't
be used anymore. Because there is another case of additional
restrictions, I proposed the binding "smbus" which means this bus is not
I2C but SMBus, so it is more restricted.

Thanks,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH v2 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Rob Herring]

On Tue, Jun 30, 2020 at 09:41:07PM +0200, Wolfram Sang wrote:
> On Thu, Jun 25, 2020 at 09:39:28AM +0200, Alain Volmat wrote:
> > Add a new binding of the i2c-stm32f7 driver to enable the handling
> > of the SMBUS-Alert.
> > 
> > The I2C/SMBUS framework already provides a mechanism to enable SMBus-Alert
> > by naming an IRQ line "smbus_alert". However, on stm32, the SMBus-Alert is
> > part of the i2c IRQ. Using the smbus_alert naming here would lead to having
> > 2 handlers (the handler of the driver and the smbus_alert handler
> > from I2C/SMBUS framework) on the unique i2c IRQ of the stm32. Meaning that
> > the smbus_alert handler would get called for all IRQ generated by the stm32
> > I2C controller.
> > 
> > For that reason, the smbus_alert IRQ naming cannot be used and a dedicated
> > binding is introduced.
> 
> What if we update the core to not register another irq handler if the
> "smbus_alert" and main irq are the same?
> 
> I think it could work. However, while trying to make a proof-of-concept,
> I found that irq descriptions in the generic i2c binding document are
> probably mixed up. And before fixing that, I'd like to get HostNotify
> done first.

Why does this even need to be in DT? Can't the driver just register that 
it supports SMBus alert or have some call to the core signaling an SMBus 
alert? 

Rob

[PATCH 15/20] dlb2: add domain alert support

[Gage Eads]

Domain alerts are a mechanism for the driver to asynchronously notify
user-space applications of device reset or hardware alarms (both to be
added in later commits). This mechanism also allows the application to
enqueue an alert to its domain, as a form of (limited) IPC in a
multi-process scenario.

An application can read its domain alerts through the domain device file's
read callback. Applications are expected to spawn a thread that performs a
blocking read, and rarely (if ever) wakes and returns to user-space.

Signed-off-by: Gage Eads 
Reviewed-by: Björn Töpel 
---
 drivers/misc/dlb2/dlb2_ioctl.c |  21 +++
 drivers/misc/dlb2/dlb2_main.c  | 134 
 drivers/misc/dlb2/dlb2_main.h  |  16 +
 include/uapi/linux/dlb2_user.h | 135 +
 4 files changed, 306 insertions(+)

diff --git a/drivers/misc/dlb2/dlb2_ioctl.c b/drivers/misc/dlb2/dlb2_ioctl.c
index a465da79..dbbf39d0b019 100644
--- a/drivers/misc/dlb2/dlb2_ioctl.c
+++ b/drivers/misc/dlb2/dlb2_ioctl.c
@@ -487,6 +487,26 @@ static int dlb2_domain_ioctl_block_on_cq_interrupt(struct 
dlb2_dev *dev,
return ret;
 }
 
+static int dlb2_domain_ioctl_enqueue_domain_alert(struct dlb2_dev *dev,
+ struct dlb2_domain *domain,
+ unsigned long user_arg,
+ u16 size)
+{
+   struct dlb2_enqueue_domain_alert_args arg;
+   int ret;
+
+   dev_dbg(dev->dlb2_device, "Entering %s()\n", __func__);
+
+   ret = dlb2_copy_from_user(dev, user_arg, size, , sizeof(arg));
+   if (ret)
+   return ret;
+
+   return dlb2_write_domain_alert(dev,
+  domain,
+  DLB2_DOMAIN_ALERT_USER,
+  arg.aux_alert_data);
+}
+
 static int dlb2_create_port_fd(struct dlb2_dev *dev,
   struct dlb2_domain *domain,
   const char *prefix,
@@ -704,6 +724,7 @@ dlb2_domain_ioctl_callback_fns[NUM_DLB2_DOMAIN_CMD] = {
dlb2_domain_ioctl_disable_ldb_port,
dlb2_domain_ioctl_disable_dir_port,
dlb2_domain_ioctl_block_on_cq_interrupt,
+   dlb2_domain_ioctl_enqueue_domain_alert,
dlb2_domain_ioctl_get_ldb_queue_depth,
dlb2_domain_ioctl_get_dir_queue_depth,
dlb2_domain_ioctl_pending_port_unmaps,
diff --git a/drivers/misc/dlb2/dlb2_main.c b/drivers/misc/dlb2/dlb2_main.c
index 33b60a8d46fe..ef964cb044f8 100644
--- a/drivers/misc/dlb2/dlb2_main.c
+++ b/drivers/misc/dlb2/dlb2_main.c
@@ -129,6 +129,9 @@ int dlb2_init_domain(struct dlb2_dev *dlb2_dev, u32 
domain_id)
kref_init(>refcnt);
domain->dlb2_dev = dlb2_dev;
 
+   spin_lock_init(>alert_lock);
+   init_waitqueue_head(>wq_head);
+
dlb2_dev->sched_domains[domain_id] = domain;
 
dlb2_dev->ops->inc_pm_refcnt(dlb2_dev->pdev, true);
@@ -259,6 +262,136 @@ static int dlb2_domain_close(struct inode *i, struct file 
*f)
return ret;
 }
 
+int dlb2_write_domain_alert(struct dlb2_dev *dev,
+   struct dlb2_domain *domain,
+   u64 alert_id,
+   u64 aux_alert_data)
+{
+   struct dlb2_domain_alert alert;
+   int idx;
+
+   if (!domain) {
+   dev_err(dev->dlb2_device,
+   "[%s()] Domain invalid (device reset)\n",
+   __func__);
+   return -EINVAL;
+   }
+
+   /* Grab the alert lock to access the read and write indexes */
+   spin_lock(>alert_lock);
+
+   /* If there's no space for this notification, return */
+   if ((domain->alert_wr_idx - domain->alert_rd_idx) ==
+   (DLB2_DOMAIN_ALERT_RING_SIZE - 1)) {
+   spin_unlock(>alert_lock);
+   return 0;
+   }
+
+   alert.alert_id = alert_id;
+   alert.aux_alert_data = aux_alert_data;
+
+   idx = domain->alert_wr_idx % DLB2_DOMAIN_ALERT_RING_SIZE;
+
+   domain->alerts[idx] = alert;
+
+   domain->alert_wr_idx++;
+
+   spin_unlock(>alert_lock);
+
+   /* Wake any blocked readers */
+   wake_up_interruptible(>wq_head);
+
+   return 0;
+}
+
+static bool dlb2_alerts_avail(struct dlb2_domain *domain)
+{
+   bool ret;
+
+   spin_lock(>alert_lock);
+
+   ret = domain->alert_rd_idx != domain->alert_wr_idx;
+
+   spin_unlock(>alert_lock);
+
+   return ret;
+}
+
+static int dlb2_read_domain_alert(struct dlb2_dev *dev,
+ struct dlb2_domain *domain,
+ struct dlb2_domain_alert *alert,
+     bool nonblock)
+{
+   int idx;
+
+   /* Grab the alert lock to access the read and write indexes */
+   

Re: [PATCH v2 0/4] stm32-f7: Addition of SMBus Alert / Host-notify features

[Wolfram Sang]

> I've just prepared the 1st new serie with only the HostNotify bits in it.
> (basically, the core part, the dt-bindings with the enable-host-notify, and
> the usage within i2c-stm32f7).

Cool, thanks!

> You mentioned in the other thread that you still have some more review comment
> I believe. Is that right ? If that is so, I'll wait for those comment and
> then push that new serie for review.

Yes, for the core part. Please wait for these comments.



signature.asc
Description: PGP signature

Re: [PATCH v2 0/4] stm32-f7: Addition of SMBus Alert / Host-notify features

[Alain Volmat]

On Tue, Jun 30, 2020 at 06:05:00PM +0200, Wolfram Sang wrote:
> On Thu, Jun 25, 2020 at 09:39:25AM +0200, Alain Volmat wrote:
> > This serie adds SMBus Alert and SMBus Host-Notify features for the 
> > i2c-stm32f7.
> 
> If it is not too much work for you, I think it makes sense to split the
> series into two, i.e. HostNotify and SMBusAlert parts.
> 

I've just prepared the 1st new serie with only the HostNotify bits in it.
(basically, the core part, the dt-bindings with the enable-host-notify, and
the usage within i2c-stm32f7).
You mentioned in the other thread that you still have some more review comment
I believe. Is that right ? If that is so, I'll wait for those comment and
then push that new serie for review.

Re: [PATCH v2 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Wolfram Sang]

On Thu, Jun 25, 2020 at 09:39:28AM +0200, Alain Volmat wrote:
> Add a new binding of the i2c-stm32f7 driver to enable the handling
> of the SMBUS-Alert.
> 
> The I2C/SMBUS framework already provides a mechanism to enable SMBus-Alert
> by naming an IRQ line "smbus_alert". However, on stm32, the SMBus-Alert is
> part of the i2c IRQ. Using the smbus_alert naming here would lead to having
> 2 handlers (the handler of the driver and the smbus_alert handler
> from I2C/SMBUS framework) on the unique i2c IRQ of the stm32. Meaning that
> the smbus_alert handler would get called for all IRQ generated by the stm32
> I2C controller.
> 
> For that reason, the smbus_alert IRQ naming cannot be used and a dedicated
> binding is introduced.

What if we update the core to not register another irq handler if the
"smbus_alert" and main irq are the same?

I think it could work. However, while trying to make a proof-of-concept,
I found that irq descriptions in the generic i2c binding document are
probably mixed up. And before fixing that, I'd like to get HostNotify
done first.

Makes sense?



signature.asc
Description: PGP signature

Re: [PATCH v2 0/4] stm32-f7: Addition of SMBus Alert / Host-notify features

[Wolfram Sang]

On Thu, Jun 25, 2020 at 09:39:25AM +0200, Alain Volmat wrote:
> This serie adds SMBus Alert and SMBus Host-Notify features for the 
> i2c-stm32f7.

If it is not too much work for you, I think it makes sense to split the
series into two, i.e. HostNotify and SMBusAlert parts.



signature.asc
Description: PGP signature

[PATCH v2 0/4] stm32-f7: Addition of SMBus Alert / Host-notify features

[Alain Volmat]

This serie adds SMBus Alert and SMBus Host-Notify features for the i2c-stm32f7.

This serie v2 rework comments from the 1st serie and replace the very generic
reg_client / unreg_client callback with HOST_NOTIFY only reg_hnotify_cli
and unreg_hnotify_cli callbacks.

Alain Volmat (4):
  i2c: smbus: add core function handling SMBus host-notify
  i2c: addition of client hnotify reg/unreg callbacks
  dt-bindings: i2c-stm32: add SMBus Alert bindings
  i2c: stm32f7: Add SMBus-specific protocols support

 .../devicetree/bindings/i2c/st,stm32-i2c.yaml  |   4 +
 drivers/i2c/busses/Kconfig |   1 +
 drivers/i2c/busses/i2c-stm32f7.c   | 194 +++--
 drivers/i2c/i2c-core-base.c|  18 +-
 drivers/i2c/i2c-core-smbus.c   | 110 
 include/linux/i2c-smbus.h  |   2 +
 include/linux/i2c.h|   8 +
 7 files changed, 325 insertions(+), 12 deletions(-)

[PATCH v2 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Alain Volmat]

Add a new binding of the i2c-stm32f7 driver to enable the handling
of the SMBUS-Alert.

The I2C/SMBUS framework already provides a mechanism to enable SMBus-Alert
by naming an IRQ line "smbus_alert". However, on stm32, the SMBus-Alert is
part of the i2c IRQ. Using the smbus_alert naming here would lead to having
2 handlers (the handler of the driver and the smbus_alert handler
from I2C/SMBUS framework) on the unique i2c IRQ of the stm32. Meaning that
the smbus_alert handler would get called for all IRQ generated by the stm32
I2C controller.

For that reason, the smbus_alert IRQ naming cannot be used and a dedicated
binding is introduced.

Signed-off-by: Alain Volmat 
---
v2: Clarify commit message

 Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
index f2fcbb361180..6fde046fae5e 100644
--- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
@@ -36,6 +36,10 @@ allOf:
 minItems: 3
 maxItems: 3
 
+    st,smbus-alert:
+  description: Enable the SMBus Alert feature
+  $ref: /schemas/types.yaml#/definitions/flag
+
   - if:
   properties:
 compatible:
-- 
2.7.4

[PATCH v3 6/6] power: supply: max17040: Support soc alert

[Iskren Chernev]

max17048 and max17049 support SOC alerts (interrupts when battery
capacity changes by +/- 1%). At the moment the driver polls for changes
every second. Using the alerts removes the need for polling.

Signed-off-by: Iskren Chernev 
---
 drivers/power/supply/max17040_battery.c | 82 ++---
 1 file changed, 73 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 54393f411211e..4425411775f26 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -25,6 +25,7 @@
 #define MAX17040_MODE  0x06
 #define MAX17040_VER   0x08
 #define MAX17040_CONFIG0x0C
+#define MAX17040_STATUS0x1A
 #define MAX17040_CMD   0xFE
 
 
@@ -33,7 +34,10 @@
 #define MAX17040_RCOMP_DEFAULT  0x9700
 
 #define MAX17040_ATHD_MASK 0x3f
+#define MAX17040_ALSC_MASK 0x40
 #define MAX17040_ATHD_DEFAULT_POWER_UP 4
+#define MAX17040_STATUS_HD_MASK0x1000
+#define MAX17040_STATUS_SC_MASK0x2000
 #define MAX17040_CFG_RCOMP_MASK0xff00
 
 enum chip_id {
@@ -55,6 +59,7 @@ struct chip_data {
u16 vcell_div;
u8  has_low_soc_alert;
u8  rcomp_bytes;
+   u8  has_soc_alert;
 };
 
 static struct chip_data max17040_family[] = {
@@ -65,6 +70,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 0,
.rcomp_bytes = 2,
+   .has_soc_alert = 0,
},
[ID_MAX17041] = {
.reset_val = 0x0054,
@@ -73,6 +79,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 0,
.rcomp_bytes = 2,
+   .has_soc_alert = 0,
},
[ID_MAX17043] = {
.reset_val = 0x0054,
@@ -81,6 +88,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17044] = {
.reset_val = 0x0054,
@@ -89,6 +97,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 1,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17048] = {
.reset_val = 0x5400,
@@ -97,6 +106,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 8,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 1,
},
[ID_MAX17049] = {
.reset_val = 0x5400,
@@ -105,6 +115,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 4,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 1,
},
[ID_MAX17058] = {
.reset_val = 0x5400,
@@ -113,6 +124,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 8,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
[ID_MAX17059] = {
.reset_val = 0x5400,
@@ -121,6 +133,7 @@ static struct chip_data max17040_family[] = {
.vcell_div = 4,
.has_low_soc_alert = 1,
.rcomp_bytes = 1,
+   .has_soc_alert = 0,
},
 };
 
@@ -156,6 +169,12 @@ static int max17040_set_low_soc_alert(struct max17040_chip 
*chip, u32 level)
MAX17040_ATHD_MASK, level);
 }
 
+static int max17040_set_soc_alert(struct max17040_chip *chip, bool enable)
+{
+   return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+   MAX17040_ALSC_MASK, enable ? MAX17040_ALSC_MASK : 0);
+}
+
 static int max17040_set_rcomp(struct max17040_chip *chip, u16 rcomp)
 {
u16 mask = chip->data.rcomp_bytes == 2 ?
@@ -298,11 +317,33 @@ static void max17040_work(struct work_struct *work)
max17040_queue_work(chip);
 }
 
+/* Returns true if alert cause was SOC change, not low SOC */
+static bool max17040_handle_soc_alert(struct max17040_chip *chip)
+{
+   bool ret = true;
+   u32 data;
+
+   regmap_read(chip->regmap, MAX17040_STATUS, );
+
+   if (data & MAX17040_STATUS_HD_MASK) {
+   // this alert was caused by low soc
+   ret = false;
+   }
+   if (data & MAX17040_STATUS_SC_MASK) {
+   // soc change bit -- deassert to mark as handled
+   regmap_write(chip->regmap, MAX17040_STATUS,
+   data & ~MAX17040_STATUS_SC_MASK);
+   }
+
+   return ret;
+}
+
 static irqreturn_t max17040_thread_handler(int id, void *dev)
 {
struct max17040_chip *chip = dev;
 
-   dev_warn(>client->dev, "IRQ: Alert batt

Re: [PATCH] ARM: exynos: update l2c_aux_mask to fix alert message

[Guillaume Tucker]

On 02/04/2020 14:11, Russell King - ARM Linux admin wrote:
> On Thu, Apr 02, 2020 at 02:03:52PM +0100, Russell King - ARM Linux admin 
> wrote:
>> On Thu, Apr 02, 2020 at 01:13:24PM +0100, Guillaume Tucker wrote:
>>> On 01/04/2020 17:31, Russell King - ARM Linux admin wrote:
>>>> On Wed, Apr 01, 2020 at 05:08:03PM +0100, Guillaume Tucker wrote:
>>>>> Allow setting the number of cycles for RAM reads in the pl310 cache
>>>>> controller L2 auxiliary control register mask (bits 0-2) since it
>>>>> needs to be changed in software.  This only affects exynos4210 and
>>>>> exynos4412 as they use the pl310 cache controller.
>>>>>
>>>>> With the mask used until now, the following warnings were generated,
>>>>> the 2nd one being a pr_alert():
>>>>>
>>>>>   L2C: platform modifies aux control register: 0x0207 -> 0x3e470001
>>>>>   L2C: platform provided aux values permit register corruption.
>>>>>
>>>>> This latency cycles value has always been set in software in spite of
>>>>> the warnings.  Keep it this way but clear the alert message about
>>>>> register corruption to acknowledge it is a valid thing to do.
>>>>
>>>> This is telling you that you are doing something you should not be
>>>> doing.  The L2C controller should be configured by board firmware
>>>> first and foremost, because if, for example, u-boot makes use of the
>>>> L2 cache, or any other pre-main kernel code (in other words,
>>>> decompressor) the setup of the L2 controller will be wrong.
>>>>
>>>> So, NAK.
>>>
>>> OK thanks, I guess I misinterpreted the meaning of the error
>>> message.  It's really saying that the register value was not the
>>> right one before the kernel tried to change it.  Next step for me
>>> is to look into U-Boot.
>>
>> The message "L2C: platform provided aux values permit register
>> corruption." means that bits are set in both the mask and the value
>> fields.  Since the new value is calculated as:
>>
>>  old = register value;
>>  new = old & mask;
>>  new |= val;
>>
>> If bits are set in both "mask" and "val" for a multi-bit field, the
>> value ending up in the field may not be what is intended.  Consider
>> a 5-bit field set initially to 10101, and the requested value is
>> 01000 with a mask of 1.  What you end up with is not 01000, but
>> 11101.  Hence, register corruption.  It is not possible to easily
>> tell whether the mask and values refer to a multi-bit field or not,
>> so the mere fact that bits are set in both issues the alert.
>>
>> The message "L2C: platform modifies aux control register ..." means
>> that you're trying to modify the value of the auxiliary control
>> register, which brings with it the problems I stated in my previous
>> email; platform configuration of the L2C must be done by firmware and
>> not the kernel for the reasons I've set out.
> 
> Actually, looking at the values there:
> 
> .l2c_aux_val= 0x3c41,
> -   .l2c_aux_mask   = 0xc20f,
> +   .l2c_aux_mask   = 0xc208,
> 
> Bit 0 is L310_AUX_CTRL_FULL_LINE_ZERO feature bit, which platforms have
> no business fiddling with - it is a Cortex-A9/L2C310 specific feature
> that needs both ends to be configured correctly to work.  The L2C code
> knows this and will deal with it.  So, .l2c_aux_val should drop setting
> bit 0.

Ack, I've just sent a patch to fix that:

  ARM: exynos: clear L310_AUX_CTRL_FULL_LINE_ZERO in default l2c_aux_val

Sorry about the confusion in my first patch, I got mislead with
the TRM of an earlier revision of the L2C-310 when these bits
were used to set the RAM data read latency.  So this all makes
sense to me now with the matching documentation for the hardware.

> It's also setting L310_AUX_CTRL_NS_LOCKDOWN, which the kernel already
> deals with - this bit should be dropped as well.

OK I'll take a look at that and send a separate patch.

Presumably this bit should also be added to the mask to report an
error if the kernel is changing it?

> It's clearing L310_AUX_CTRL_CACHE_REPLACE_RR - this should be setup by
> firmware.

As far as I can tell, L310_AUX_CTRL_CACHE_REPLACE_RR is bit 25
i.e. 0x0200, which is set in hardware by default and is not
changed by the kernel:

L2C: platform modifies aux control register: 0x0207 -> 0x3e470001

Also this bit is already in the mask, and there is no error about
register corruption any more with the patch I sent today.

> For the prefetching, I thought there were DT properties for that.
> Please look at that, and see whether you can eliminate most of the
> .l2c_aux_val field set bits, and the .l2c_aux_mask clear bits.

Ack, it's handled by l2c310_of_parse().  I'll take a look and see
if I can make another patch for that.

Thanks,
Guillaume

Re: [PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Alain Volmat]

On Sat, May 23, 2020 at 10:36:01AM +, w...@kernel.org wrote:
> 
> > > > +    st,smbus-alert:
> > > > +  description: Enable the SMBus Alert feature
> > > > +  $ref: /schemas/types.yaml#/definitions/flag
> > > > +
> > > 
> > > We already have smbus_alert interrupt. Can't you just check for this in 
> > > the slave nodes and enable if found?
> > 
> > My understanding reading the code (smbalert_probe within i2c-smbus.c, 
> > of_i2c_setup_smbus_alert called when
> > registering an adapter within i2c-core-smbus.c) is that smbus_alert refers 
> > to an interrupt on the
> > adapter side. That is an interrupt that would be triggered when the adapter 
> > is receiving an smbus_alert
> > message.
> > In our case (stm32f7), we do not have specific interrupt for that purpose. 
> > The interrupt triggered when
> > an SMBUS Alert is received (by the adapter) is the same interrupt as for 
> > other reasons and we check
> > within the irq handler within stm32f7 the reason before calling 
> > i2c_handle_smbus_alert if the status
> > register indicated an SMBUS Alert.
> > So my understanding is that we cannot rely on the mechanism of naming an 
> > interrupt smbus_alert.
> > Did I misunderstood something ?
> 
> I just wonder what is bad about specifying the same interrupt twice in
> the interrupt properties? You could then check in probe if "smbus_alert"
> is populated and if it matches the main irq.
> 

Here's my understanding of the current implementation.
During the adapter registration, the function of_i2c_setup_smbus_alert is called
and if a interrupt is named "smbus_alert" in the adapter node, a new device
"smbus_alert" will be created. This will leads to smbalert_probe to be called,
and a request_irq will be done with the irq value read from the adapter node.
This means that we will have both our handle (the handler of the main irq
of the stm32 i2c driver) and the smbus_alert handler on the same irq. Leading
to smbus_alert being called everytime there is a irq (most of the time not
smbus_alert related) coming from the stm32_i2c. (since this is our main irq).

So to me this approach can't work. I'd understand if the smbus_alert property
was on the client node in the same way as it is done for host-notify however
that's not the case.
This is why I was proposing to have our own st,smbus-alert property to decide
to enable or not the smbus_alert. In our case, we cannot rely on the mechanism
done by of_i2c_setup_smbus_alert since for us, smbus_alert irq is just one
case of all the other stm32 i2c irq. (this is the same irq, and we check after
by reading the interrupt status register).

Re: [PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[w...@kernel.org]

> > > +st,smbus-alert:
> > > +  description: Enable the SMBus Alert feature
> > > +  $ref: /schemas/types.yaml#/definitions/flag
> > > +
> > 
> > We already have smbus_alert interrupt. Can't you just check for this in 
> > the slave nodes and enable if found?
> 
> My understanding reading the code (smbalert_probe within i2c-smbus.c, 
> of_i2c_setup_smbus_alert called when
> registering an adapter within i2c-core-smbus.c) is that smbus_alert refers to 
> an interrupt on the
> adapter side. That is an interrupt that would be triggered when the adapter 
> is receiving an smbus_alert
> message.
> In our case (stm32f7), we do not have specific interrupt for that purpose. 
> The interrupt triggered when
> an SMBUS Alert is received (by the adapter) is the same interrupt as for 
> other reasons and we check
> within the irq handler within stm32f7 the reason before calling 
> i2c_handle_smbus_alert if the status
> register indicated an SMBUS Alert.
> So my understanding is that we cannot rely on the mechanism of naming an 
> interrupt smbus_alert.
> Did I misunderstood something ?

I just wonder what is bad about specifying the same interrupt twice in
the interrupt properties? You could then check in probe if "smbus_alert"
is populated and if it matches the main irq.



signature.asc
Description: PGP signature

Re: [PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Alain Volmat]

Gentle Reminder, as I wrote in my previous responce, smbus_alert interrupt
refers to an host and not a client. And since we do not have a dedicated
irq for smbus_alert, I propose to add this st, binding to enable the
smbus_alert mechanism.

On Wed, May 13, 2020 at 07:42:31AM +0200, Alain Volmat wrote:
> Hello Rob,
> 
> On Wed, May 13, 2020 at 02:19:32AM +, Rob Herring wrote:
> > On Tue, May 05, 2020 at 07:51:10AM +0200, Alain Volmat wrote:
> > > Add a new binding of the i2c-stm32f7 driver to enable the handling
> > > of the SMBUS-Alert
> > > 
> > > Signed-off-by: Alain Volmat 
> > > ---
> > >  Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
> > > b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> > > index b50a2f420b36..04c0882c3661 100644
> > > --- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> > > +++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> > > @@ -36,6 +36,10 @@ allOf:
> > >  minItems: 3
> > >  maxItems: 3
> > >  
> > > +st,smbus-alert:
> > > +  description: Enable the SMBus Alert feature
> > > +  $ref: /schemas/types.yaml#/definitions/flag
> > > +
> > 
> > We already have smbus_alert interrupt. Can't you just check for this in 
> > the slave nodes and enable if found?
> 
> My understanding reading the code (smbalert_probe within i2c-smbus.c, 
> of_i2c_setup_smbus_alert called when
> registering an adapter within i2c-core-smbus.c) is that smbus_alert refers to 
> an interrupt on the
> adapter side. That is an interrupt that would be triggered when the adapter 
> is receiving an smbus_alert
> message.
> In our case (stm32f7), we do not have specific interrupt for that purpose. 
> The interrupt triggered when
> an SMBUS Alert is received (by the adapter) is the same interrupt as for 
> other reasons and we check
> within the irq handler within stm32f7 the reason before calling 
> i2c_handle_smbus_alert if the status
> register indicated an SMBUS Alert.
> So my understanding is that we cannot rely on the mechanism of naming an 
> interrupt smbus_alert.
> Did I misunderstood something ?
> 
> > 
> > >- if:
> > >properties:
> > >  compatible:
> > > -- 
> > > 2.17.1
> > > 

Re: [PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Alain Volmat]

Hello Rob,

On Wed, May 13, 2020 at 02:19:32AM +, Rob Herring wrote:
> On Tue, May 05, 2020 at 07:51:10AM +0200, Alain Volmat wrote:
> > Add a new binding of the i2c-stm32f7 driver to enable the handling
> > of the SMBUS-Alert
> > 
> > Signed-off-by: Alain Volmat 
> > ---
> >  Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
> > b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> > index b50a2f420b36..04c0882c3661 100644
> > --- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> > @@ -36,6 +36,10 @@ allOf:
> >      minItems: 3
> >      maxItems: 3
> >  
> > +st,smbus-alert:
> > +  description: Enable the SMBus Alert feature
> > +  $ref: /schemas/types.yaml#/definitions/flag
> > +
> 
> We already have smbus_alert interrupt. Can't you just check for this in 
> the slave nodes and enable if found?

My understanding reading the code (smbalert_probe within i2c-smbus.c, 
of_i2c_setup_smbus_alert called when
registering an adapter within i2c-core-smbus.c) is that smbus_alert refers to 
an interrupt on the
adapter side. That is an interrupt that would be triggered when the adapter is 
receiving an smbus_alert
message.
In our case (stm32f7), we do not have specific interrupt for that purpose. The 
interrupt triggered when
an SMBUS Alert is received (by the adapter) is the same interrupt as for other 
reasons and we check
within the irq handler within stm32f7 the reason before calling 
i2c_handle_smbus_alert if the status
register indicated an SMBUS Alert.
So my understanding is that we cannot rely on the mechanism of naming an 
interrupt smbus_alert.
Did I misunderstood something ?

> 
> >- if:
> >properties:
> >  compatible:
> > -- 
> > 2.17.1
> > 

Re: [PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Rob Herring]

On Tue, May 05, 2020 at 07:51:10AM +0200, Alain Volmat wrote:
> Add a new binding of the i2c-stm32f7 driver to enable the handling
> of the SMBUS-Alert
> 
> Signed-off-by: Alain Volmat 
> ---
>  Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
> b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> index b50a2f420b36..04c0882c3661 100644
> --- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
> @@ -36,6 +36,10 @@ allOf:
>  minItems: 3
>      maxItems: 3
>  
> +st,smbus-alert:
> +  description: Enable the SMBus Alert feature
> +  $ref: /schemas/types.yaml#/definitions/flag
> +

We already have smbus_alert interrupt. Can't you just check for this in 
the slave nodes and enable if found?

>- if:
>properties:
>  compatible:
> -- 
> 2.17.1
> 

[PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings

[Alain Volmat]

Add a new binding of the i2c-stm32f7 driver to enable the handling
of the SMBUS-Alert

Signed-off-by: Alain Volmat 
---
 Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml 
b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
index b50a2f420b36..04c0882c3661 100644
--- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
@@ -36,6 +36,10 @@ allOf:
 minItems: 3
 maxItems: 3
 
+st,smbus-alert:
+  description: Enable the SMBus Alert feature
+  $ref: /schemas/types.yaml#/definitions/flag
+
   - if:
   properties:
 compatible:
-- 
2.17.1

[PATCH 0/4] stm32-f7: Addition of SMBus Alert / Host-notify features

[Alain Volmat]

This serie adds SMBus Alert and SMBus Host-Notify features for the i2c-stm32f7.

For that purpore, I propose two enhancements to the i2c framework.
1. Addition of host-notify client handling as part of the
i2c-core-smbus so that any other i2c adapter can benefit from it,
even those without specific HW support for Host-Notify.
2. Addition of two i2c_algorithm reg_client and unreg_client allowing
adapter to take some actions when a client is being probed.
Indeed, in case of host-notify, the binding/flag is related to the
client and the adapter might want to enable the host-notify feature
only when there is a client requesting the host-notify.
(Since SMBus host-notify is using the address 0x08 which is a valid
I2C address, keeping host-notify always enabled would make the 0x08
I2C slave address non usable).

Alain Volmat (4):
  i2c: smbus: add core function handling SMBus host-notify
  i2c: addition of client reg/unreg callbacks
  dt-bindings: i2c-stm32: add SMBus Alert bindings
  i2c: stm32f7: Add SMBus-specific protocols support

 .../devicetree/bindings/i2c/st,stm32-i2c.yaml |   4 +
 drivers/i2c/busses/Kconfig|   1 +
 drivers/i2c/busses/i2c-stm32f7.c  | 198 +-
 drivers/i2c/i2c-core-base.c   |  11 +
 drivers/i2c/i2c-core-smbus.c  | 105 ++
 include/linux/i2c-smbus.h |   2 +
 include/linux/i2c.h   |   6 +
 7 files changed, 317 insertions(+), 10 deletions(-)

-- 
2.17.1

Re: [PATCH v3 3/5] power: supply: max17040: Config alert SOC low level threshold from FDT

[Krzysztof Kozlowski]

On Mon, 3 Jun 2019 at 00:42, Matheus Castello  wrote:
>
>
>
> On 5/29/19 11:46 AM, Krzysztof Kozlowski wrote:
> > On Mon, 27 May 2019 at 04:46, Matheus Castello  
> > wrote:
> >>
> >> For configuration of fuel gauge alert for a low level state of charge
> >> interrupt we add a function to config level threshold and a device tree
> >> binding property to set it in flatned device tree node.
> >>
> >> Now we can use "maxim,alert-low-soc-level" property with the values from
> >> 1% up to 32% to configure alert interrupt threshold.
> >>
> >> Signed-off-by: Matheus Castello 
> >> ---
> >>   drivers/power/supply/max17040_battery.c | 52 +++--
> >>   1 file changed, 49 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/power/supply/max17040_battery.c 
> >> b/drivers/power/supply/max17040_battery.c
> >> index b7433e9ca7c2..2f4851608cfe 100644
> >> --- a/drivers/power/supply/max17040_battery.c
> >> +++ b/drivers/power/supply/max17040_battery.c
> >> @@ -29,6 +29,9 @@
> >>   #define MAX17040_DELAY 1000
> >>   #define MAX17040_BATTERY_FULL  95
> >>
> >> +#define MAX17040_ATHD_MASK 0xFFC0
> >> +#define MAX17040_ATHD_DEFAULT_POWER_UP 4
> >> +
> >>   struct max17040_chip {
> >>  struct i2c_client   *client;
> >>  struct delayed_work work;
> >> @@ -43,6 +46,8 @@ struct max17040_chip {
> >>  int soc;
> >>  /* State Of Charge */
> >>  int status;
> >> +   /* Low alert threshold from 32% to 1% of the State of Charge */
> >> +   u32 low_soc_alert_threshold;
> >>   };
> >>
> >>   static int max17040_get_property(struct power_supply *psy,
> >> @@ -99,6 +104,28 @@ static void max17040_reset(struct i2c_client *client)
> >>  max17040_write_reg(client, MAX17040_CMD, 0x0054);
> >>   }
> >>
> >> +static int max17040_set_low_soc_threshold_alert(struct i2c_client *client,
> >> +   u32 level)
> >> +{
> >> +   int ret;
> >> +   u16 data;
> >> +
> >> +   /* check if level is between 1% and 32% */
> >> +   if (level > 0 && level < 33) {
> >> +   level = 32 - level;
> >> +   data = max17040_read_reg(client, MAX17040_RCOMP);
> >> +   /* clear the alrt bit and set LSb 5 bits */
> >> +   data &= MAX17040_ATHD_MASK;
> >> +   data |= level;
> >> +   max17040_write_reg(client, MAX17040_RCOMP, data);
> >> +   ret = 0;
>
> I will put the return of max17040_write_reg on ret, instead of ret = 0.
>
> >> +   } else {
> >> +   ret = -EINVAL;
> >> +   }
> >
> > This is unusual way of handling error... when you parse DTS, you
> > accept any value for "level" (even incorrect one). You validate the
> > value later when setting it and show an error... however you ignore
> > the error of max17040_write_reg() here... This is correct but looks
> > unusual.
> >
>
> Ok, so would it be better to check the level value in
> "max17040_get_of_data" and return an error there if the input is wrong?

I think yes. It looks more natural - validate the value as early as
possible and fail the probe which gives the information about point of
failure.

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On Sun, 2 Jun 2019 at 23:42, Matheus Castello  wrote:
>
> > On Mon, 27 May 2019 at 04:45, Matheus Castello  
> > wrote:
> >>
> >> For configure low level state of charge threshold alert signaled from
> >> max17040 we add "maxim,alert-low-soc-level" property.
> >>
> >> Signed-off-by: Matheus Castello 
> >> ---
> >>   .../power/supply/max17040_battery.txt | 28 +++
> >>   1 file changed, 28 insertions(+)
> >>   create mode 100644 
> >> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> >> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> >> new file mode 100644
> >> index ..a13e8d50ff7b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> >> @@ -0,0 +1,28 @@
> >> +max17040_battery
> >> +~~~~
> >> +
> >> +Required properties :
> >> + - compatible : "maxim,max17040" or "maxim,max77836-battery"
> >
> > One more comment. The datasheet for max17040 says that there is on
> > ALERT pin and ALERT bits in RCOMP register. Which device are you
> > using? If it turns out that max17040 does not support it, then the
> > driver and bindings should reflect this - interrupts should not be set
> > on max17040.
> >
>
> Yes you are right, max17040 have no ALERT pin. I am using max17043. Let
> me know what you think would be best, put a note about it in the
> description, add a compatibles like "maxim,max17043" and
> "maxim,max17044"? What do you think?

Usually in such case driver should behave differently for different
devices. This difference is chosen by compatible. Since there is
already max77836 compatible - which has the ALERT pin (probably it
just includes 17043 fuel gauge) - you can customize it. No need for
new compatible, unless it stops working on max77836...

Anyway, configuring interrupts on max17040 would be probably harmless
but still it is not really correct. The registers should not be
touched in such case.

Best regards,
Krzysztof

Re: [PATCH v3 3/5] power: supply: max17040: Config alert SOC low level threshold from FDT

[Matheus Castello]

On 5/29/19 11:46 AM, Krzysztof Kozlowski wrote:

On Mon, 27 May 2019 at 04:46, Matheus Castello  wrote:


For configuration of fuel gauge alert for a low level state of charge
interrupt we add a function to config level threshold and a device tree
binding property to set it in flatned device tree node.

Now we can use "maxim,alert-low-soc-level" property with the values from
1% up to 32% to configure alert interrupt threshold.

Signed-off-by: Matheus Castello 
---
  drivers/power/supply/max17040_battery.c | 52 +++--
  1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index b7433e9ca7c2..2f4851608cfe 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -29,6 +29,9 @@
  #define MAX17040_DELAY 1000
  #define MAX17040_BATTERY_FULL  95

+#define MAX17040_ATHD_MASK 0xFFC0
+#define MAX17040_ATHD_DEFAULT_POWER_UP 4
+
  struct max17040_chip {
 struct i2c_client   *client;
 struct delayed_work work;
@@ -43,6 +46,8 @@ struct max17040_chip {
 int soc;
 /* State Of Charge */
 int status;
+   /* Low alert threshold from 32% to 1% of the State of Charge */
+   u32 low_soc_alert_threshold;
  };

  static int max17040_get_property(struct power_supply *psy,
@@ -99,6 +104,28 @@ static void max17040_reset(struct i2c_client *client)
 max17040_write_reg(client, MAX17040_CMD, 0x0054);
  }

+static int max17040_set_low_soc_threshold_alert(struct i2c_client *client,
+   u32 level)
+{
+   int ret;
+   u16 data;
+
+   /* check if level is between 1% and 32% */
+   if (level > 0 && level < 33) {
+   level = 32 - level;
+   data = max17040_read_reg(client, MAX17040_RCOMP);
+   /* clear the alrt bit and set LSb 5 bits */
+   data &= MAX17040_ATHD_MASK;
+   data |= level;
+   max17040_write_reg(client, MAX17040_RCOMP, data);
+   ret = 0;


I will put the return of max17040_write_reg on ret, instead of ret = 0.


+   } else {
+   ret = -EINVAL;
+   }


This is unusual way of handling error... when you parse DTS, you
accept any value for "level" (even incorrect one). You validate the
value later when setting it and show an error... however you ignore
the error of max17040_write_reg() here... This is correct but looks
unusual.



Ok, so would it be better to check the level value in 
"max17040_get_of_data" and return an error there if the input is wrong?


Best Regards,
Matheus Castello


Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Matheus Castello]

On Mon, 27 May 2019 at 04:45, Matheus Castello  wrote:


For configure low level state of charge threshold alert signaled from
max17040 we add "maxim,alert-low-soc-level" property.

Signed-off-by: Matheus Castello 
---
  .../power/supply/max17040_battery.txt | 28 +++
  1 file changed, 28 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

diff --git 
a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index ..a13e8d50ff7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,28 @@
+max17040_battery
+
+
+Required properties :
+ - compatible : "maxim,max17040" or "maxim,max77836-battery"


One more comment. The datasheet for max17040 says that there is on
ALERT pin and ALERT bits in RCOMP register. Which device are you
using? If it turns out that max17040 does not support it, then the
driver and bindings should reflect this - interrupts should not be set
on max17040.



Yes you are right, max17040 have no ALERT pin. I am using max17043. Let 
me know what you think would be best, put a note about it in the 
description, add a compatibles like "maxim,max17043" and 
"maxim,max17044"? What do you think?


Best Regards,
Matheus Castello


Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On Mon, 27 May 2019 at 04:45, Matheus Castello  wrote:
>
> For configure low level state of charge threshold alert signaled from
> max17040 we add "maxim,alert-low-soc-level" property.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../power/supply/max17040_battery.txt | 28 +++
>  1 file changed, 28 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> new file mode 100644
> index ..a13e8d50ff7b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -0,0 +1,28 @@
> +max17040_battery
> +
> +
> +Required properties :
> + - compatible : "maxim,max17040" or "maxim,max77836-battery"

One more comment. The datasheet for max17040 says that there is on
ALERT pin and ALERT bits in RCOMP register. Which device are you
using? If it turns out that max17040 does not support it, then the
driver and bindings should reflect this - interrupts should not be set
on max17040.

Best regards,
Krzysztof

Re: [PATCH v3 3/5] power: supply: max17040: Config alert SOC low level threshold from FDT

[Krzysztof Kozlowski]

On Mon, 27 May 2019 at 04:46, Matheus Castello  wrote:
>
> For configuration of fuel gauge alert for a low level state of charge
> interrupt we add a function to config level threshold and a device tree
> binding property to set it in flatned device tree node.
>
> Now we can use "maxim,alert-low-soc-level" property with the values from
> 1% up to 32% to configure alert interrupt threshold.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 52 +++--
>  1 file changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index b7433e9ca7c2..2f4851608cfe 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -29,6 +29,9 @@
>  #define MAX17040_DELAY 1000
>  #define MAX17040_BATTERY_FULL  95
>
> +#define MAX17040_ATHD_MASK 0xFFC0
> +#define MAX17040_ATHD_DEFAULT_POWER_UP 4
> +
>  struct max17040_chip {
> struct i2c_client   *client;
> struct delayed_work work;
> @@ -43,6 +46,8 @@ struct max17040_chip {
> int soc;
> /* State Of Charge */
> int status;
> +   /* Low alert threshold from 32% to 1% of the State of Charge */
> +   u32 low_soc_alert_threshold;
>  };
>
>  static int max17040_get_property(struct power_supply *psy,
> @@ -99,6 +104,28 @@ static void max17040_reset(struct i2c_client *client)
> max17040_write_reg(client, MAX17040_CMD, 0x0054);
>  }
>
> +static int max17040_set_low_soc_threshold_alert(struct i2c_client *client,
> +   u32 level)
> +{
> +   int ret;
> +   u16 data;
> +
> +   /* check if level is between 1% and 32% */
> +   if (level > 0 && level < 33) {
> +   level = 32 - level;
> +   data = max17040_read_reg(client, MAX17040_RCOMP);
> +   /* clear the alrt bit and set LSb 5 bits */
> +   data &= MAX17040_ATHD_MASK;
> +   data |= level;
> +   max17040_write_reg(client, MAX17040_RCOMP, data);
> +   ret = 0;
> +   } else {
> +   ret = -EINVAL;
> +   }

This is unusual way of handling error... when you parse DTS, you
accept any value for "level" (even incorrect one). You validate the
value later when setting it and show an error... however you ignore
the error of max17040_write_reg() here... This is correct but looks
unusual.

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On Mon, 27 May 2019 at 04:45, Matheus Castello  wrote:
>
> For configure low level state of charge threshold alert signaled from
> max17040 we add "maxim,alert-low-soc-level" property.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../power/supply/max17040_battery.txt | 28 +++
>  1 file changed, 28 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> new file mode 100644
> index ..a13e8d50ff7b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -0,0 +1,28 @@
> +max17040_battery
> +
> +
> +Required properties :
> + - compatible : "maxim,max17040" or "maxim,max77836-battery"
> +
> +Optional properties :
> +- maxim,alert-low-soc-level :  The alert threshold that sets the state of
> +   charge level (%) where an interrupt is
> +   generated. Can be configured from 1 up to 32
> +   (%). If skipped the power up default value of
> +   4 (%) will be used.
> +- interrupts : Interrupt line see 
> Documentation/devicetree/
> +   bindings/interrupt-controller/interrupts.txt

Based on driver's behavior, I understand that these two properties
come in pair so maxim,alert-low-soc-level (or its default value) will
be used if and only if interrupts property is present. Maybe mention
this? In general looks fine to me:
Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

> +- wakeup-source :  This device has wakeup capabilities. Use this
> +   property to use alert low SOC level interrupt
> +   as wake up source.
> +
> +Example:
> +
> +   battery-fuel-gauge@36 {
> +   compatible = "maxim,max17040";
> +   reg = <0x36>;
> +   maxim,alert-low-soc-level = <10>;
> +   interrupt-parent = <>;
> +   interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> +   wakeup-source;
> +   };
> --
> 2.20.1
>

Re: [PATCH v3 1/5] power: supply: max17040: Add IRQ handler for low SOC alert

[Krzysztof Kozlowski]

On Mon, 27 May 2019 at 04:45, Matheus Castello  wrote:
>
> According datasheet max17040 has a pin for alert host for low SOC.
> This pin can be used as external interrupt, so we need to check for
> interrupts assigned for device and handle it.
>
> In handler we are checking and storing fuel gauge registers values
> and send an uevent to notificate user space, so user space can decide
> save work or turn off since the alert demonstrate that the battery may
> no have the power to keep the system turned on for much longer.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 65 +++--
>  1 file changed, 60 insertions(+), 5 deletions(-)

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

[PATCH v3 1/5] power: supply: max17040: Add IRQ handler for low SOC alert

[Matheus Castello]

According datasheet max17040 has a pin for alert host for low SOC.
This pin can be used as external interrupt, so we need to check for
interrupts assigned for device and handle it.

In handler we are checking and storing fuel gauge registers values
and send an uevent to notificate user space, so user space can decide
save work or turn off since the alert demonstrate that the battery may
no have the power to keep the system turned on for much longer.

Signed-off-by: Matheus Castello 
---
 drivers/power/supply/max17040_battery.c | 65 +++--
 1 file changed, 60 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 91cafc7bed30..b7433e9ca7c2 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -160,21 +161,40 @@ static void max17040_get_status(struct i2c_client *client)
chip->status = POWER_SUPPLY_STATUS_FULL;
 }

+static void max17040_check_changes(struct i2c_client *client)
+{
+   max17040_get_vcell(client);
+   max17040_get_soc(client);
+   max17040_get_online(client);
+   max17040_get_status(client);
+}
+
 static void max17040_work(struct work_struct *work)
 {
struct max17040_chip *chip;

chip = container_of(work, struct max17040_chip, work.work);
-
-   max17040_get_vcell(chip->client);
-   max17040_get_soc(chip->client);
-   max17040_get_online(chip->client);
-   max17040_get_status(chip->client);
+   max17040_check_changes(chip->client);

queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
 }

+static irqreturn_t max17040_thread_handler(int id, void *dev)
+{
+   struct max17040_chip *chip = dev;
+   struct i2c_client *client = chip->client;
+
+   dev_warn(>dev, "IRQ: Alert battery low level");
+   /* read registers */
+   max17040_check_changes(chip->client);
+
+   /* send uevent */
+   power_supply_changed(chip->battery);
+
+   return IRQ_HANDLED;
+}
+
 static enum power_supply_property max17040_battery_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
@@ -220,6 +240,25 @@ static int max17040_probe(struct i2c_client *client,
max17040_reset(client);
max17040_get_version(client);

+   /* check interrupt */
+   if (client->irq) {
+   int ret;
+   unsigned int flags;
+
+   dev_info(>dev, "IRQ: enabled\n");
+   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
+   ret = devm_request_threaded_irq(>dev, client->irq, NULL,
+   max17040_thread_handler, flags,
+   chip->battery->desc->name,
+   chip);
+
+   if (ret) {
+   client->irq = 0;
+   dev_warn(>dev,
+   "Failed to get IRQ err %d\n", ret);
+   }
+   }
+
INIT_DEFERRABLE_WORK(>work, max17040_work);
queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
@@ -244,6 +283,14 @@ static int max17040_suspend(struct device *dev)
struct max17040_chip *chip = i2c_get_clientdata(client);

cancel_delayed_work(>work);
+
+   if (client->irq) {
+   if (device_may_wakeup(dev))
+   enable_irq_wake(client->irq);
+   else
+   disable_irq_wake(client->irq);
+   }
+
return 0;
 }

@@ -254,6 +301,14 @@ static int max17040_resume(struct device *dev)

queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
+
+   if (client->irq) {
+   if (device_may_wakeup(dev))
+   disable_irq_wake(client->irq);
+   else
+   enable_irq_wake(client->irq);
+   }
+
return 0;
 }

--
2.20.1

[PATCH v3 3/5] power: supply: max17040: Config alert SOC low level threshold from FDT

[Matheus Castello]

For configuration of fuel gauge alert for a low level state of charge
interrupt we add a function to config level threshold and a device tree
binding property to set it in flatned device tree node.

Now we can use "maxim,alert-low-soc-level" property with the values from
1% up to 32% to configure alert interrupt threshold.

Signed-off-by: Matheus Castello 
---
 drivers/power/supply/max17040_battery.c | 52 +++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index b7433e9ca7c2..2f4851608cfe 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -29,6 +29,9 @@
 #define MAX17040_DELAY 1000
 #define MAX17040_BATTERY_FULL  95

+#define MAX17040_ATHD_MASK 0xFFC0
+#define MAX17040_ATHD_DEFAULT_POWER_UP 4
+
 struct max17040_chip {
struct i2c_client   *client;
struct delayed_work work;
@@ -43,6 +46,8 @@ struct max17040_chip {
int soc;
/* State Of Charge */
int status;
+   /* Low alert threshold from 32% to 1% of the State of Charge */
+   u32 low_soc_alert_threshold;
 };

 static int max17040_get_property(struct power_supply *psy,
@@ -99,6 +104,28 @@ static void max17040_reset(struct i2c_client *client)
max17040_write_reg(client, MAX17040_CMD, 0x0054);
 }

+static int max17040_set_low_soc_threshold_alert(struct i2c_client *client,
+   u32 level)
+{
+   int ret;
+   u16 data;
+
+   /* check if level is between 1% and 32% */
+   if (level > 0 && level < 33) {
+   level = 32 - level;
+   data = max17040_read_reg(client, MAX17040_RCOMP);
+   /* clear the alrt bit and set LSb 5 bits */
+   data &= MAX17040_ATHD_MASK;
+   data |= level;
+   max17040_write_reg(client, MAX17040_RCOMP, data);
+   ret = 0;
+   } else {
+   ret = -EINVAL;
+   }
+
+   return ret;
+}
+
 static void max17040_get_vcell(struct i2c_client *client)
 {
struct max17040_chip *chip = i2c_get_clientdata(client);
@@ -161,6 +188,16 @@ static void max17040_get_status(struct i2c_client *client)
chip->status = POWER_SUPPLY_STATUS_FULL;
 }

+static void max17040_get_of_data(struct max17040_chip *chip)
+{
+   struct device *dev = >client->dev;
+   struct device_node *np = dev->of_node;
+
+   if (of_property_read_u32(np, "maxim,alert-low-soc-level",
+   >low_soc_alert_threshold))
+   chip->low_soc_alert_threshold = MAX17040_ATHD_DEFAULT_POWER_UP;
+}
+
 static void max17040_check_changes(struct i2c_client *client)
 {
max17040_get_vcell(client);
@@ -226,6 +263,7 @@ static int max17040_probe(struct i2c_client *client,

chip->client = client;
chip->pdata = client->dev.platform_data;
+   max17040_get_of_data(chip);

i2c_set_clientdata(client, chip);
psy_cfg.drv_data = chip;
@@ -243,12 +281,20 @@ static int max17040_probe(struct i2c_client *client,
/* check interrupt */
if (client->irq) {
int ret;
-   unsigned int flags;
+
+   ret = max17040_set_low_soc_threshold_alert(client,
+   chip->low_soc_alert_threshold);
+   if (ret) {
+   dev_err(>dev,
+   "Failed to set low SOC alert: err %d\n",
+   ret);
+   return ret;
+   }

dev_info(>dev, "IRQ: enabled\n");
-   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
ret = devm_request_threaded_irq(>dev, client->irq, NULL,
-   max17040_thread_handler, flags,
+   max17040_thread_handler,
+   (client->flags | IRQF_ONESHOT),
chip->battery->desc->name,
chip);

--
2.20.1

[PATCH v3 2/5] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Matheus Castello]

For configure low level state of charge threshold alert signaled from
max17040 we add "maxim,alert-low-soc-level" property.

Signed-off-by: Matheus Castello 
---
 .../power/supply/max17040_battery.txt | 28 +++
 1 file changed, 28 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

diff --git 
a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index ..a13e8d50ff7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,28 @@
+max17040_battery
+
+
+Required properties :
+ - compatible : "maxim,max17040" or "maxim,max77836-battery"
+
+Optional properties :
+- maxim,alert-low-soc-level :  The alert threshold that sets the state of
+   charge level (%) where an interrupt is
+   generated. Can be configured from 1 up to 32
+   (%). If skipped the power up default value of
+   4 (%) will be used.
+- interrupts : Interrupt line see 
Documentation/devicetree/
+   bindings/interrupt-controller/interrupts.txt
+- wakeup-source :  This device has wakeup capabilities. Use this
+   property to use alert low SOC level interrupt
+   as wake up source.
+
+Example:
+
+   battery-fuel-gauge@36 {
+   compatible = "maxim,max17040";
+   reg = <0x36>;
+   maxim,alert-low-soc-level = <10>;
+   interrupt-parent = <>;
+   interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+   wakeup-source;
+   };
--
2.20.1

[PATCH v3 0/5] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Matheus Castello]

This series add IRQ handler for low level SOC alert, define a devicetree
binding attribute to configure the alert level threshold and check for
changes in SOC and power supply status for send uevents.

Max17040 have a pin for alert host about low level state of charge and
this alert can be configured in a threshold from 1% up to 32% of SOC.

Tested on Toradex Colibri iMX7D, with a SparkFun Lipo Fuel Gauge module
based on MAXIM MAX17043.

Thanks Krzysztof and Rob for yours time reviewing it. Let me know what
you think about the fixes.

Krzysztof I added a new patch to the series to check the battery charge up
and clear ALRT bit when the SOC is above alert threshold, so not generating
duplicate interrupts.

Changes since v1:
(Suggested by Krzysztof Kozlowski)
- Put common code from max17040_work and max17040_thread_handler in a function
- Code style fixes
- Define mask and low level threshold alert default
- Check return value from max17040_set_soc_threshold
- Set low level state of charge threshold before IRQ
- CC maintainers from drivers/mfd/max14577
- Use flags from FDT client->flags instead hard coded it
- Mention interrupts on DT Documentation
- Fix "maxim,max77836-battery" missed from DT Documentation
- Fix commit description

Changes since v2:
(Suggested by Krzysztof Kozlowski)
- Fix ebusy exception
- Remove device_init_wakeup from probe, let wakeup-source from DT decide that
- Fix the use of "charger" to fuel-gauge on device tree description
- Clear ALRT bit while setting the SOC threshold
- Check SOC and clear ALRT bit when the SOC are above alert threshold
- Fix unnecessary uevent when SOC is an ERRNO
- Notify user space when power supply status change

(Suggested by Rob Herring)
- Fix the use of "charger" to fuel-gauge on device tree description
- Add the (%) units on the description of property
- Drop interrupt-parent
- Fix name of property to let clear that is a low level SOC alert

Matheus Castello (5):
  power: supply: max17040: Add IRQ handler for low SOC alert
  dt-bindings: power: supply: Max17040: Add low level SOC alert
threshold
  power: supply: max17040: Config alert SOC low level threshold from FDT
  power: supply: max17040: Clear ALRT bit when the SOC are above
threshold
  power: supply: max17040: Send uevent in SOC and status change

 .../power/supply/max17040_battery.txt |  28 
 drivers/power/supply/max17040_battery.c   | 134 +-
 2 files changed, 158 insertions(+), 4 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

--
2.20.1

Re: [PATCH v2 2/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Matheus Castello]

On 4/29/19 7:13 PM, Rob Herring wrote:

On Sun, Apr 14, 2019 at 10:26:33PM -0300, Matheus Castello wrote:

For configure low level state of charge threshold alert signaled from
max17040 we add "maxim,alert-soc-level" property.

Signed-off-by: Matheus Castello 
---
  .../power/supply/max17040_battery.txt | 24 +++
  1 file changed, 24 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

diff --git 
a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index ..9b2cc67d556f
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,24 @@
+max17040_battery
+
+
+Required properties :
+ - compatible : "maxim,max17040" or "maxim,max77836-battery"


This is really a charger, not a battery.



max17040 is a fuel gauge, max77836 MUIC has it integrated. Because of 
this we use it in the compatible list.



+
+Optional properties :
+- maxim,alert-soc-level :  The alert threshold that sets the state of
+   charge level where an interrupt is generated.
+   Can be configured from 1 up to 32. If skipped
+   the power up default value of 4 will be used.


Units? This is a low or high alert? Does a common property make sense
here?



It is a low level alert. I will change the name of the property to 
"maxim,alert-low-soc-level" to make this clear and I will put the 
percent unit in the description.


I do not find any common property that I can use here, if I am wrong let 
me know.



+- interrupt-parent :   The GPIO bank from the interrupt line.


Drop this. interrupt-parent is implied.


Ok, I will do.




+- interrupts : Interrupt line see 
Documentation/devicetree/
+   bindings/interrupt-controller/interrupts.txt
+
+Example:
+
+   battery-charger@36 {
+   compatible = "maxim,max17040";
+   reg = <0x36>;
+   maxim,alert-soc-level = <10>;
+   interrupt-parent = <>;
+   interrupts = <2 IRQ_TYPE_EDGE_FALLING>;


Usually there are battery properties that need to be described too...



I will fix this for "battery-fuel-gauge@36". I will also add the 
description for wake-source as optional property.


Thanks.

Best Regards,
Matheus Castello


+   };
--
2.17.0

Re: [PATCH v2 2/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Rob Herring]

On Sun, Apr 14, 2019 at 10:26:33PM -0300, Matheus Castello wrote:
> For configure low level state of charge threshold alert signaled from
> max17040 we add "maxim,alert-soc-level" property.
> 
> Signed-off-by: Matheus Castello 
> ---
>  .../power/supply/max17040_battery.txt | 24 +++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> new file mode 100644
> index ..9b2cc67d556f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -0,0 +1,24 @@
> +max17040_battery
> +
> +
> +Required properties :
> + - compatible : "maxim,max17040" or "maxim,max77836-battery"

This is really a charger, not a battery.

> +
> +Optional properties :
> +- maxim,alert-soc-level :The alert threshold that sets the state of
> + charge level where an interrupt is generated.
> + Can be configured from 1 up to 32. If skipped
> + the power up default value of 4 will be used.

Units? This is a low or high alert? Does a common property make sense 
here?

> +- interrupt-parent : The GPIO bank from the interrupt line.

Drop this. interrupt-parent is implied.

> +- interrupts :   Interrupt line see 
> Documentation/devicetree/
> + bindings/interrupt-controller/interrupts.txt
> +
> +Example:
> +
> + battery-charger@36 {
> + compatible = "maxim,max17040";
> + reg = <0x36>;
> + maxim,alert-soc-level = <10>;
> + interrupt-parent = <>;
> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;

Usually there are battery properties that need to be described too...

> + };
> -- 
> 2.17.0
> 

Re: [PATCH v2 1/4] power: supply: max17040: Add IRQ handler for low SOC alert

[Matheus Castello]

Em 4/15/19 4:10 AM, Krzysztof Kozlowski escreveu:

On Mon, 15 Apr 2019 at 03:49, Matheus Castello  wrote:


According datasheet max17040 has a pin for alert host for low SOC.
This pin can be used as external interrupt, so we need to check for
interrupts assigned for device and handle it.

In hadler we are checking and storing fuel gauge registers values
and send an uevent to notificate user space, so user space can decide
save work or turn off since the alert demonstrate that the battery may
no have the power to keep the system turned on for much longer.

Signed-off-by: Matheus Castello 
---
  drivers/power/supply/max17040_battery.c | 69 +++--
  1 file changed, 64 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 91cafc7bed30..8d2f8ed3f44c 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -13,6 +13,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -160,21 +161,40 @@ static void max17040_get_status(struct i2c_client *client)
 chip->status = POWER_SUPPLY_STATUS_FULL;
  }

+static void max17040_check_changes(struct i2c_client *client)
+{
+   max17040_get_vcell(client);
+   max17040_get_soc(client);
+   max17040_get_online(client);
+   max17040_get_status(client);
+}
+
  static void max17040_work(struct work_struct *work)
  {
 struct max17040_chip *chip;

 chip = container_of(work, struct max17040_chip, work.work);
-
-   max17040_get_vcell(chip->client);
-   max17040_get_soc(chip->client);
-   max17040_get_online(chip->client);
-   max17040_get_status(chip->client);
+   max17040_check_changes(chip->client);

 queue_delayed_work(system_power_efficient_wq, >work,
MAX17040_DELAY);
  }

+static irqreturn_t max17040_thread_handler(int id, void *dev)
+{
+   struct max17040_chip *chip = dev;
+   struct i2c_client *client = chip->client;
+
+   dev_warn(>dev, "IRQ: Alert battery low level");
+   /* read registers */
+   max17040_check_changes(chip->client);
+
+   /* send uevent */
+   power_supply_changed(chip->battery);
+
+   return IRQ_HANDLED;
+}
+
  static enum power_supply_property max17040_battery_props[] = {
 POWER_SUPPLY_PROP_STATUS,
 POWER_SUPPLY_PROP_ONLINE,
@@ -217,6 +237,27 @@ static int max17040_probe(struct i2c_client *client,
 return PTR_ERR(chip->battery);
 }

+   /* check interrupt */
+   if (client->irq) {
+   int ret;
+   unsigned int flags;
+
+   dev_info(>dev, "IRQ: enabled\n");
+   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
+
+   ret = devm_request_threaded_irq(>dev, client->irq, NULL,
+   max17040_thread_handler, flags,
+   chip->battery->desc->name,
+   chip);
+
+   if (ret) {
+   client->irq = 0;
+   if (ret != -EBUSY)


Why not on EBUSY?


+   dev_warn(>dev,
+   "Failed to get IRQ err %d\n", ret);
+   }
+   }
+
 max17040_reset(client);
 max17040_get_version(client);

@@ -224,6 +265,8 @@ static int max17040_probe(struct i2c_client *client,
 queue_delayed_work(system_power_efficient_wq, >work,
MAX17040_DELAY);

+   device_init_wakeup(>dev, 1);


Either you parse DT for wakeup-source property and use it... or you
unconditionally enable wakeup. In the second case - there is no point
to check later for device_may_wakeup(). Instead check the return value
of device_init_wakeup().


+
 return 0;
  }

@@ -244,6 +287,14 @@ static int max17040_suspend(struct device *dev)
 struct max17040_chip *chip = i2c_get_clientdata(client);

 cancel_delayed_work(>work);
+
+   if (client->irq) {
+   if (device_may_wakeup(dev))
+   enable_irq_wake(client->irq);
+   else
+   disable_irq_wake(client->irq);


Did you try the wakeup from suspend? You do not have a disable_irq()
here which usually was needed for interrupts to work properly on
suspend. Maybe this was fixed?

Best regards,
Krzysztof


Hi Krzysztof,

I test it using mem state and suspend, resuming seems to have worked 
correctly ...


Thanks for the review, I'm working in your suggestions and I expect to 
send v3 this weekend.


Best Regards,
Matheus Castello

Re: [PATCH v2 3/4] power: supply: max17040: Config alert SOC low level threshold from FDT

[Krzysztof Kozlowski]

On Mon, 15 Apr 2019 at 03:51, Matheus Castello  wrote:
>
> For configuration of fuel gauge alert for a low level state of charge
> interrupt we add a function to config level threshold and a device tree
> binding property to set it in flatned device tree node.
>
> Now we can use "maxim,alert-soc-level" property with the values from
> 1 up to 32 to configure alert interrupt threshold.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 56 ++---
>  1 file changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index 8d2f8ed3f44c..f036f272d52f 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -29,6 +29,9 @@
>  #define MAX17040_DELAY 1000
>  #define MAX17040_BATTERY_FULL  95
>
> +#define MAX17040_ATHD_MASK 0xFFE0
> +#define MAX17040_ATHD_DEFAULT_POWER_UP 4
> +
>  struct max17040_chip {
> struct i2c_client   *client;
> struct delayed_work work;
> @@ -43,6 +46,8 @@ struct max17040_chip {
> int soc;
> /* State Of Charge */
> int status;
> +   /* Alert threshold from 32% to 1% of the State of Charge */
> +   u32 alert_threshold;
>  };
>
>  static int max17040_get_property(struct power_supply *psy,
> @@ -119,6 +124,27 @@ static void max17040_get_soc(struct i2c_client *client)
> chip->soc = (soc >> 8);
>  }
>
> +static int max17040_set_soc_threshold(struct i2c_client *client, u32 level)
> +{
> +   int ret;
> +   u16 data;
> +
> +   /* check if level is between 1% and 32% */
> +   if (level > 0 && level < 33) {
> +   /* alert threshold use LSb 5 bits from RCOMP */
> +   level = 32 - level;
> +   data = max17040_read_reg(client, MAX17040_RCOMP);
> +   data &= MAX17040_ATHD_MASK;
> +   data |= level;
> +   max17040_write_reg(client, MAX17040_RCOMP, data);
> +   ret = 0;
> +   } else {
> +   ret = -EINVAL;
> +   }
> +
> +   return ret;
> +}
> +
>  static void max17040_get_version(struct i2c_client *client)
>  {
> u16 version;
> @@ -161,6 +187,16 @@ static void max17040_get_status(struct i2c_client 
> *client)
> chip->status = POWER_SUPPLY_STATUS_FULL;
>  }
>
> +static void max17040_get_of_data(struct max17040_chip *chip)
> +{
> +   struct device *dev = >client->dev;
> +   struct device_node *np = dev->of_node;
> +
> +   if (of_property_read_u32(np, "maxim,alert-soc-level",
> +   >alert_threshold))
> +   chip->alert_threshold = MAX17040_ATHD_DEFAULT_POWER_UP;
> +}
> +
>  static void max17040_check_changes(struct i2c_client *client)
>  {
> max17040_get_vcell(client);
> @@ -226,6 +262,7 @@ static int max17040_probe(struct i2c_client *client,
>
> chip->client = client;
> chip->pdata = client->dev.platform_data;
> +   max17040_get_of_data(chip);
>
> i2c_set_clientdata(client, chip);
> psy_cfg.drv_data = chip;
> @@ -237,16 +274,26 @@ static int max17040_probe(struct i2c_client *client,
> return PTR_ERR(chip->battery);
> }
>
> +   max17040_reset(client);
> +   max17040_get_version(client);
> +
> /* check interrupt */
> if (client->irq) {
> int ret;
> -   unsigned int flags;
> +
> +   ret = max17040_set_soc_threshold(client, 
> chip->alert_threshold);
> +   if (ret) {
> +   dev_err(>dev,
> +   "Failed to set SOC alert threshold: err %d\n",
> +   ret);
> +   return ret;
> +   }
>
> dev_info(>dev, "IRQ: enabled\n");
> -   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
>
> ret = devm_request_threaded_irq(>dev, client->irq, 
> NULL,
> -   max17040_thread_handler, 
> flags,
> +   max17040_thread_handler,
> +   (client->flags | 
> IRQF_ONESHOT),
> chip->battery->desc->name,
> chip);

Now I noticed that you are not clearing the ALRT status bit. Therefore
alert interrupt will be generated only once. Even if SoC goes up
(charged) and then down to alert level it will not be generated second
time. At least documentation for max77836 is saying that clearing this
bit is required.

Best regards,
Krzysztof

Re: [PATCH v2 2/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On Mon, 15 Apr 2019 at 03:50, Matheus Castello  wrote:
>
> For configure low level state of charge threshold alert signaled from
> max17040 we add "maxim,alert-soc-level" property.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../power/supply/max17040_battery.txt | 24 +++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> new file mode 100644
> index ..9b2cc67d556f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -0,0 +1,24 @@
> +max17040_battery
> +
> +
> +Required properties :
> + - compatible : "maxim,max17040" or "maxim,max77836-battery"
> +
> +Optional properties :
> +- maxim,alert-soc-level :  The alert threshold that sets the state of
> +   charge level where an interrupt is generated.
> +   Can be configured from 1 up to 32. If skipped
> +   the power up default value of 4 will be used.
> +- interrupt-parent :   The GPIO bank from the interrupt line.

It does not have to be GPIO so:
"phandle of the parent interrupt controller"

> +- interrupts : Interrupt line see 
> Documentation/devicetree/
> +   bindings/interrupt-controller/interrupts.txt
> +
> +Example:
> +
> +   battery-charger@36 {

It is a fuel gauge, not battery charger.

Best regards,
Krzysztof

> +   compatible = "maxim,max17040";
> +   reg = <0x36>;
> +   maxim,alert-soc-level = <10>;
> +   interrupt-parent = <>;
> +   interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> +   };
> --
> 2.17.0
>

Re: [PATCH v2 1/4] power: supply: max17040: Add IRQ handler for low SOC alert

[Krzysztof Kozlowski]

On Mon, 15 Apr 2019 at 03:49, Matheus Castello  wrote:
>
> According datasheet max17040 has a pin for alert host for low SOC.
> This pin can be used as external interrupt, so we need to check for
> interrupts assigned for device and handle it.
>
> In hadler we are checking and storing fuel gauge registers values
> and send an uevent to notificate user space, so user space can decide
> save work or turn off since the alert demonstrate that the battery may
> no have the power to keep the system turned on for much longer.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 69 +++--
>  1 file changed, 64 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index 91cafc7bed30..8d2f8ed3f44c 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -160,21 +161,40 @@ static void max17040_get_status(struct i2c_client 
> *client)
> chip->status = POWER_SUPPLY_STATUS_FULL;
>  }
>
> +static void max17040_check_changes(struct i2c_client *client)
> +{
> +   max17040_get_vcell(client);
> +   max17040_get_soc(client);
> +   max17040_get_online(client);
> +   max17040_get_status(client);
> +}
> +
>  static void max17040_work(struct work_struct *work)
>  {
> struct max17040_chip *chip;
>
> chip = container_of(work, struct max17040_chip, work.work);
> -
> -   max17040_get_vcell(chip->client);
> -   max17040_get_soc(chip->client);
> -   max17040_get_online(chip->client);
> -   max17040_get_status(chip->client);
> +   max17040_check_changes(chip->client);
>
> queue_delayed_work(system_power_efficient_wq, >work,
>MAX17040_DELAY);
>  }
>
> +static irqreturn_t max17040_thread_handler(int id, void *dev)
> +{
> +   struct max17040_chip *chip = dev;
> +   struct i2c_client *client = chip->client;
> +
> +   dev_warn(>dev, "IRQ: Alert battery low level");
> +   /* read registers */
> +   max17040_check_changes(chip->client);
> +
> +   /* send uevent */
> +   power_supply_changed(chip->battery);
> +
> +   return IRQ_HANDLED;
> +}
> +
>  static enum power_supply_property max17040_battery_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> @@ -217,6 +237,27 @@ static int max17040_probe(struct i2c_client *client,
> return PTR_ERR(chip->battery);
> }
>
> +   /* check interrupt */
> +   if (client->irq) {
> +   int ret;
> +   unsigned int flags;
> +
> +   dev_info(>dev, "IRQ: enabled\n");
> +   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
> +
> +   ret = devm_request_threaded_irq(>dev, client->irq, 
> NULL,
> +   max17040_thread_handler, 
> flags,
> +   chip->battery->desc->name,
> +   chip);
> +
> +   if (ret) {
> +   client->irq = 0;
> +   if (ret != -EBUSY)

Why not on EBUSY?

> +   dev_warn(>dev,
> +   "Failed to get IRQ err %d\n", ret);
> +   }
> +   }
> +
> max17040_reset(client);
> max17040_get_version(client);
>
> @@ -224,6 +265,8 @@ static int max17040_probe(struct i2c_client *client,
> queue_delayed_work(system_power_efficient_wq, >work,
>MAX17040_DELAY);
>
> +   device_init_wakeup(>dev, 1);

Either you parse DT for wakeup-source property and use it... or you
unconditionally enable wakeup. In the second case - there is no point
to check later for device_may_wakeup(). Instead check the return value
of device_init_wakeup().

> +
> return 0;
>  }
>
> @@ -244,6 +287,14 @@ static int max17040_suspend(struct device *dev)
> struct max17040_chip *chip = i2c_get_clientdata(client);
>
> cancel_delayed_work(>work);
> +
> +   if (client->irq) {
> +   if (device_may_wakeup(dev))
> +   enable_irq_wake(client->irq);
> +   else
> +   disable_irq_wake(client->irq);

Did you try the wakeup from suspend? You do not have a disable_irq()
here which usually was needed for interrupts to work properly on
suspend. Maybe this was fixed?

Best regards,
Krzysztof

[PATCH v2 1/4] power: supply: max17040: Add IRQ handler for low SOC alert

[Matheus Castello]

According datasheet max17040 has a pin for alert host for low SOC.
This pin can be used as external interrupt, so we need to check for
interrupts assigned for device and handle it.

In hadler we are checking and storing fuel gauge registers values
and send an uevent to notificate user space, so user space can decide
save work or turn off since the alert demonstrate that the battery may
no have the power to keep the system turned on for much longer.

Signed-off-by: Matheus Castello 
---
 drivers/power/supply/max17040_battery.c | 69 +++--
 1 file changed, 64 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 91cafc7bed30..8d2f8ed3f44c 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -160,21 +161,40 @@ static void max17040_get_status(struct i2c_client *client)
chip->status = POWER_SUPPLY_STATUS_FULL;
 }
 
+static void max17040_check_changes(struct i2c_client *client)
+{
+   max17040_get_vcell(client);
+   max17040_get_soc(client);
+   max17040_get_online(client);
+   max17040_get_status(client);
+}
+
 static void max17040_work(struct work_struct *work)
 {
struct max17040_chip *chip;
 
chip = container_of(work, struct max17040_chip, work.work);
-
-   max17040_get_vcell(chip->client);
-   max17040_get_soc(chip->client);
-   max17040_get_online(chip->client);
-   max17040_get_status(chip->client);
+   max17040_check_changes(chip->client);
 
queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
 }
 
+static irqreturn_t max17040_thread_handler(int id, void *dev)
+{
+   struct max17040_chip *chip = dev;
+   struct i2c_client *client = chip->client;
+
+   dev_warn(>dev, "IRQ: Alert battery low level");
+   /* read registers */
+   max17040_check_changes(chip->client);
+
+   /* send uevent */
+   power_supply_changed(chip->battery);
+
+   return IRQ_HANDLED;
+}
+
 static enum power_supply_property max17040_battery_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
@@ -217,6 +237,27 @@ static int max17040_probe(struct i2c_client *client,
return PTR_ERR(chip->battery);
}
 
+   /* check interrupt */
+   if (client->irq) {
+   int ret;
+   unsigned int flags;
+
+   dev_info(>dev, "IRQ: enabled\n");
+   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
+
+   ret = devm_request_threaded_irq(>dev, client->irq, NULL,
+   max17040_thread_handler, flags,
+   chip->battery->desc->name,
+   chip);
+
+   if (ret) {
+   client->irq = 0;
+   if (ret != -EBUSY)
+   dev_warn(>dev,
+   "Failed to get IRQ err %d\n", ret);
+   }
+   }
+
max17040_reset(client);
max17040_get_version(client);
 
@@ -224,6 +265,8 @@ static int max17040_probe(struct i2c_client *client,
queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
 
+   device_init_wakeup(>dev, 1);
+
return 0;
 }
 
@@ -244,6 +287,14 @@ static int max17040_suspend(struct device *dev)
struct max17040_chip *chip = i2c_get_clientdata(client);
 
cancel_delayed_work(>work);
+
+   if (client->irq) {
+   if (device_may_wakeup(dev))
+   enable_irq_wake(client->irq);
+   else
+   disable_irq_wake(client->irq);
+   }
+
return 0;
 }
 
@@ -254,6 +305,14 @@ static int max17040_resume(struct device *dev)
 
queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
+
+   if (client->irq) {
+   if (device_may_wakeup(dev))
+   disable_irq_wake(client->irq);
+   else
+   enable_irq_wake(client->irq);
+   }
+
return 0;
 }
 
-- 
2.17.0

[PATCH v2 2/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Matheus Castello]

For configure low level state of charge threshold alert signaled from
max17040 we add "maxim,alert-soc-level" property.

Signed-off-by: Matheus Castello 
---
 .../power/supply/max17040_battery.txt | 24 +++
 1 file changed, 24 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

diff --git 
a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index ..9b2cc67d556f
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,24 @@
+max17040_battery
+
+
+Required properties :
+ - compatible : "maxim,max17040" or "maxim,max77836-battery"
+
+Optional properties :
+- maxim,alert-soc-level :  The alert threshold that sets the state of
+   charge level where an interrupt is generated.
+   Can be configured from 1 up to 32. If skipped
+   the power up default value of 4 will be used.
+- interrupt-parent :   The GPIO bank from the interrupt line.
+- interrupts : Interrupt line see 
Documentation/devicetree/
+   bindings/interrupt-controller/interrupts.txt
+
+Example:
+
+   battery-charger@36 {
+   compatible = "maxim,max17040";
+   reg = <0x36>;
+   maxim,alert-soc-level = <10>;
+   interrupt-parent = <>;
+   interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+   };
-- 
2.17.0

[PATCH v2 3/4] power: supply: max17040: Config alert SOC low level threshold from FDT

[Matheus Castello]

For configuration of fuel gauge alert for a low level state of charge
interrupt we add a function to config level threshold and a device tree
binding property to set it in flatned device tree node.

Now we can use "maxim,alert-soc-level" property with the values from
1 up to 32 to configure alert interrupt threshold.

Signed-off-by: Matheus Castello 
---
 drivers/power/supply/max17040_battery.c | 56 ++---
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 8d2f8ed3f44c..f036f272d52f 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -29,6 +29,9 @@
 #define MAX17040_DELAY 1000
 #define MAX17040_BATTERY_FULL  95
 
+#define MAX17040_ATHD_MASK 0xFFE0
+#define MAX17040_ATHD_DEFAULT_POWER_UP 4
+
 struct max17040_chip {
struct i2c_client   *client;
struct delayed_work work;
@@ -43,6 +46,8 @@ struct max17040_chip {
int soc;
/* State Of Charge */
int status;
+   /* Alert threshold from 32% to 1% of the State of Charge */
+   u32 alert_threshold;
 };
 
 static int max17040_get_property(struct power_supply *psy,
@@ -119,6 +124,27 @@ static void max17040_get_soc(struct i2c_client *client)
chip->soc = (soc >> 8);
 }
 
+static int max17040_set_soc_threshold(struct i2c_client *client, u32 level)
+{
+   int ret;
+   u16 data;
+
+   /* check if level is between 1% and 32% */
+   if (level > 0 && level < 33) {
+   /* alert threshold use LSb 5 bits from RCOMP */
+   level = 32 - level;
+   data = max17040_read_reg(client, MAX17040_RCOMP);
+   data &= MAX17040_ATHD_MASK;
+   data |= level;
+   max17040_write_reg(client, MAX17040_RCOMP, data);
+   ret = 0;
+   } else {
+   ret = -EINVAL;
+   }
+
+   return ret;
+}
+
 static void max17040_get_version(struct i2c_client *client)
 {
u16 version;
@@ -161,6 +187,16 @@ static void max17040_get_status(struct i2c_client *client)
chip->status = POWER_SUPPLY_STATUS_FULL;
 }
 
+static void max17040_get_of_data(struct max17040_chip *chip)
+{
+   struct device *dev = >client->dev;
+   struct device_node *np = dev->of_node;
+
+   if (of_property_read_u32(np, "maxim,alert-soc-level",
+   >alert_threshold))
+   chip->alert_threshold = MAX17040_ATHD_DEFAULT_POWER_UP;
+}
+
 static void max17040_check_changes(struct i2c_client *client)
 {
max17040_get_vcell(client);
@@ -226,6 +262,7 @@ static int max17040_probe(struct i2c_client *client,
 
chip->client = client;
chip->pdata = client->dev.platform_data;
+   max17040_get_of_data(chip);
 
i2c_set_clientdata(client, chip);
psy_cfg.drv_data = chip;
@@ -237,16 +274,26 @@ static int max17040_probe(struct i2c_client *client,
return PTR_ERR(chip->battery);
}
 
+   max17040_reset(client);
+   max17040_get_version(client);
+
/* check interrupt */
if (client->irq) {
int ret;
-   unsigned int flags;
+
+   ret = max17040_set_soc_threshold(client, chip->alert_threshold);
+   if (ret) {
+   dev_err(>dev,
+   "Failed to set SOC alert threshold: err %d\n",
+   ret);
+   return ret;
+   }
 
dev_info(>dev, "IRQ: enabled\n");
-   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
 
ret = devm_request_threaded_irq(>dev, client->irq, NULL,
-   max17040_thread_handler, flags,
+   max17040_thread_handler,
+   (client->flags | IRQF_ONESHOT),
chip->battery->desc->name,
chip);
 
@@ -258,9 +305,6 @@ static int max17040_probe(struct i2c_client *client,
}
}
 
-   max17040_reset(client);
-   max17040_get_version(client);
-
INIT_DEFERRABLE_WORK(>work, max17040_work);
queue_delayed_work(system_power_efficient_wq, >work,
   MAX17040_DELAY);
-- 
2.17.0

[PATCH v2 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Matheus Castello]

This series add IRQ handler for low level SOC alert, define a devicetree 
binding attribute to configure the alert level threshold and check for
changes in SOC for send uevents.

Max17040 have a pin for alert host about low level state of charge and
this alert can be configured in a threshold from 1% up to 32% of SOC.

Tested on Toradex Colibri iMX7D, with a SparkFun Lipo Fuel Gauge module
based on MAXIM MAX17043.

Thanks Krzysztof Kozlowski for your time reviewing it, and forgive me for
the delay in working on it, now I'm back to the patchs. Let me know what
you think about the fixes and I'm open to maintainers suggestions.

Changes since v1:
(Suggested by Krzysztof Kozlowski)
- Put common code from max17040_work and max17040_thread_handler in a function
- Code style fixes
- Define mask and low level threshold alert default
- Check return value from max17040_set_soc_threshold
- Set low level state of charge threshold before IRQ
- CC maintainers from drivers/mfd/max14577
- Use flags from FDT client->flags instead hard coded it
- Mention interrupts on DT Documentation
- Fix "maxim,max77836-battery" missed from DT Documentation
- Fix commit description

Matheus Castello (4):
  power: supply: max17040: Add IRQ handler for low SOC alert
  dt-bindings: power: supply: Max17040: Add low level SOC alert
threshold
  power: supply: max17040: Config alert SOC low level threshold from FDT
  power: supply: max17040: Send uevent in SOC changes

 .../power/supply/max17040_battery.txt |  24 
 drivers/power/supply/max17040_battery.c   | 118 +-
 2 files changed, 138 insertions(+), 4 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

-- 
2.17.0

[PATCH 4.9 17/20] btrfs: Remove false alert when fiemap range is smaller than on-disk extent

[Greg Kroah-Hartman]

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Qu Wenruo 

commit 848c23b78fafdcd3270b06a30737f8dbd70c347f upstream.

Commit 4751832da990 ("btrfs: fiemap: Cache and merge fiemap extent before
submit it to user") introduced a warning to catch unemitted cached
fiemap extent.

However such warning doesn't take the following case into consideration:

0   4K  8K
|< fiemap range --->|
|<--- On-disk extent -->|

In this case, the whole 0~8K is cached, and since it's larger than
fiemap range, it break the fiemap extent emit loop.
This leaves the fiemap extent cached but not emitted, and caught by the
final fiemap extent sanity check, causing kernel warning.

This patch removes the kernel warning and renames the sanity check to
emit_last_fiemap_cache() since it's possible and valid to have cached
fiemap extent.

Reported-by: David Sterba 
Reported-by: Adam Borowski 
Fixes: 4751832da990 ("btrfs: fiemap: Cache and merge fiemap extent ...")
Signed-off-by: Qu Wenruo 
Signed-off-by: David Sterba 
Signed-off-by: Greg Kroah-Hartman 

---
 fs/btrfs/extent_io.c |   28 
 1 file changed, 12 insertions(+), 16 deletions(-)

--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4463,29 +4463,25 @@ try_submit_last:
 }
 
 /*
- * Sanity check for fiemap cache
+ * Emit last fiemap cache
  *
- * All fiemap cache should be submitted by emit_fiemap_extent()
- * Iteration should be terminated either by last fiemap extent or
- * fieinfo->fi_extents_max.
- * So no cached fiemap should exist.
+ * The last fiemap cache may still be cached in the following case:
+ * 0 4k8k
+ * |<- Fiemap range ->|
+ * |<  First extent --->|
+ *
+ * In this case, the first extent range will be cached but not emitted.
+ * So we must emit it before ending extent_fiemap().
  */
-static int check_fiemap_cache(struct btrfs_fs_info *fs_info,
-  struct fiemap_extent_info *fieinfo,
-  struct fiemap_cache *cache)
+static int emit_last_fiemap_cache(struct btrfs_fs_info *fs_info,
+ struct fiemap_extent_info *fieinfo,
+ struct fiemap_cache *cache)
 {
int ret;
 
if (!cache->cached)
return 0;
 
-   /* Small and recoverbale problem, only to info developer */
-#ifdef CONFIG_BTRFS_DEBUG
-   WARN_ON(1);
-#endif
-   btrfs_warn(fs_info,
-  "unhandled fiemap cache detected: offset=%llu phys=%llu 
len=%llu flags=0x%x",
-  cache->offset, cache->phys, cache->len, cache->flags);
ret = fiemap_fill_next_extent(fieinfo, cache->offset, cache->phys,
  cache->len, cache->flags);
cache->cached = false;
@@ -4701,7 +4697,7 @@ int extent_fiemap(struct inode *inode, s
}
 out_free:
if (!ret)
-   ret = check_fiemap_cache(root->fs_info, fieinfo, );
+   ret = emit_last_fiemap_cache(root->fs_info, fieinfo, );
free_extent_map(em);
 out:
btrfs_free_path(path);

[PATCH v2 7/9] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Amit Kucheria]

75 degrees is too aggressive for throttling the CPU. After speaking to
Qualcomm engineers, increase it to 95 degrees.

Signed-off-by: Amit Kucheria 
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0e659b0524ba..fb7da678b116 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1720,7 +1720,7 @@
 
trips {
cpu_alert0: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1741,7 +1741,7 @@
 
trips {
cpu_alert1: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1762,7 +1762,7 @@
 
trips {
cpu_alert2: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1783,7 +1783,7 @@
 
trips {
cpu_alert3: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1804,7 +1804,7 @@
 
trips {
cpu_alert4: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1825,7 +1825,7 @@
 
trips {
cpu_alert5: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1846,7 +1846,7 @@
 
trips {
cpu_alert6: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1867,7 +1867,7 @@
 
trips {
cpu_alert7: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
-- 
2.17.1

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Matthias Kaehlcke]

On Fri, Jan 11, 2019 at 03:54:23PM +0530, Amit Kucheria wrote:
> On Thu, Jan 10, 2019 at 6:45 AM Matthias Kaehlcke  wrote:
> >
> > Hi Amit,
> >
> > On Thu, Jan 10, 2019 at 05:30:55AM +0530, Amit Kucheria wrote:
> > > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > > Qualcomm engineers, increase it to 95 degrees.
> > >
> > > Signed-off-by: Amit Kucheria 
> > > ---
> > >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > index c27cbd3bcb0a..29e823b0caf4 100644
> > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > @@ -1692,7 +1692,7 @@
> > >
> > >   trips {
> > >   cpu_alert0: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1713,7 +1713,7 @@
> > >
> > >   trips {
> > >   cpu_alert1: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1734,7 +1734,7 @@
> > >
> > >   trips {
> > >   cpu_alert2: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1755,7 +1755,7 @@
> > >
> > >   trips {
> > >   cpu_alert3: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1776,7 +1776,7 @@
> > >
> > >   trips {
> > >   cpu_alert4: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1797,7 +1797,7 @@
> > >
> > >   trips {
> > >   cpu_alert5: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1818,7 +1818,7 @@
> > >
> > >   trips {
> > >   cpu_alert6: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> > > @@ -1839,7 +1839,7 @@
> > >
> > >   trips {
> > >   cpu_alert7: trip0 {
> > > - temperature = <75000>;
> > > + temperature = <95000>;
> > >   hysteresis = <2000>;
> > >   type = "passive";
> > >   };
> >
> > The change itself looks good to me, however I wonder if it would be
> > worth to eliminate redundancy and merge the current 8 thermal zones
> > into 2, one for the Silver and one for the Gold cluster (as done by
> > http://crrev.com/c/1381752). There is a single cooling device for
> > each cluster, so it's not clear to me if there is any gain from having
> > a separate thermal zone for each CPU. If it is important to monitor
> > the temperatures of the individual cores this can still be done by
> > configuring the thermal zone of the cluster with multiple thermal
> > sensors.
> 
> Reducing the number of thermal zones to 2 (by grouping 4 sensors per
> zone) is not possible due a limitation of the thermal framework[1]. It
> is something that we want to address. Previous attempts to fix this
> were rejected 

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Amit Kucheria]

On Thu, Jan 10, 2019 at 6:45 AM Matthias Kaehlcke  wrote:
>
> Hi Amit,
>
> On Thu, Jan 10, 2019 at 05:30:55AM +0530, Amit Kucheria wrote:
> > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > Qualcomm engineers, increase it to 95 degrees.
> >
> > Signed-off-by: Amit Kucheria 
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index c27cbd3bcb0a..29e823b0caf4 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -1692,7 +1692,7 @@
> >
> >   trips {
> >   cpu_alert0: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1713,7 +1713,7 @@
> >
> >   trips {
> >   cpu_alert1: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1734,7 +1734,7 @@
> >
> >   trips {
> >   cpu_alert2: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1755,7 +1755,7 @@
> >
> >   trips {
> >   cpu_alert3: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1776,7 +1776,7 @@
> >
> >   trips {
> >   cpu_alert4: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1797,7 +1797,7 @@
> >
> >   trips {
> >   cpu_alert5: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1818,7 +1818,7 @@
> >
> >   trips {
> >   cpu_alert6: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
> > @@ -1839,7 +1839,7 @@
> >
> >   trips {
> >   cpu_alert7: trip0 {
> > - temperature = <75000>;
> > + temperature = <95000>;
> >   hysteresis = <2000>;
> >   type = "passive";
> >   };
>
> The change itself looks good to me, however I wonder if it would be
> worth to eliminate redundancy and merge the current 8 thermal zones
> into 2, one for the Silver and one for the Gold cluster (as done by
> http://crrev.com/c/1381752). There is a single cooling device for
> each cluster, so it's not clear to me if there is any gain from having
> a separate thermal zone for each CPU. If it is important to monitor
> the temperatures of the individual cores this can still be done by
> configuring the thermal zone of the cluster with multiple thermal
> sensors.

Reducing the number of thermal zones to 2 (by grouping 4 sensors per
zone) is not possible due a limitation of the thermal framework[1]. It
is something that we want to address. Previous attempts to fix this
were rejected for various reasons. Eduardo was going to share a way to
have more flexible mapping between sensors and zones after discussions
at LPC.

 Eduardo, do you have anything we can review?  :-)

Having said that, we'll need some aggregation functions when we add
multiple sensors 

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Viresh Kumar]

On 10-01-19, 12:00, Matthias Kaehlcke wrote:
> Viresh helped me understand that we currently need to add cooling
> device entries for all CPUs to the DT, even though at most one will be
> active per freq domain at any time (I wonder if this could be changed
> though).

Actually we were only adding cooling-cells in CPU0 until now and I
fixed that, so that is going to stay :)

The idea is that the hardware should be described properly and not
partially. Even if all the CPUs are part of the same freq-domain, they
are all capable of being a cooling device here and the DT should
describe that. Kernel will ofcourse create a single cooling device.

-- 
viresh

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Amit Kucheria]

On Thu, Jan 10, 2019 at 5:59 AM Stephen Boyd  wrote:
>
> Quoting Amit Kucheria (2019-01-09 16:00:55)
> > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > Qualcomm engineers, increase it to 95 degrees.
> >
> > Signed-off-by: Amit Kucheria 
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
>
> Is the plan that these are some defaults that would be adjusted by board
> variants? Just curious why we have anything in here and don't punt it
> all to each board dts file.

These values are meant to be safe values for silicon operation as
characterised[1] by the HW team. So I'd consider them a more than just
default values. It also gives future boards a safe starting point.

IMHO it would be better to have the characterized values merged
upstream and if really required, those values can be tweaked in the
board-specific DTS.

[1] Caveat: The characterisation probably focused on mobile devices
and might change depending on form-factor, active cooling and heat
dissipation design but those differences should only impact the skin
temperature, not the operation of the SoC itself.

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Matthias Kaehlcke]

On Fri, Jan 11, 2019 at 01:15:09AM +0530, Amit Kucheria wrote:
> On Thu, Jan 10, 2019 at 7:45 AM Matthias Kaehlcke  wrote:
> >
> > On Wed, Jan 09, 2019 at 05:15:33PM -0800, Matthias Kaehlcke wrote:
> > > Hi Amit,
> > >
> > > On Thu, Jan 10, 2019 at 05:30:55AM +0530, Amit Kucheria wrote:
> > > > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > > > Qualcomm engineers, increase it to 95 degrees.
> > > >
> > > > Signed-off-by: Amit Kucheria 
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
> > > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> > > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > index c27cbd3bcb0a..29e823b0caf4 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > @@ -1692,7 +1692,7 @@
> > > >
> > > > trips {
> > > > cpu_alert0: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1713,7 +1713,7 @@
> > > >
> > > > trips {
> > > > cpu_alert1: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1734,7 +1734,7 @@
> > > >
> > > > trips {
> > > > cpu_alert2: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1755,7 +1755,7 @@
> > > >
> > > > trips {
> > > > cpu_alert3: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1776,7 +1776,7 @@
> > > >
> > > > trips {
> > > > cpu_alert4: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1797,7 +1797,7 @@
> > > >
> > > > trips {
> > > > cpu_alert5: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1818,7 +1818,7 @@
> > > >
> > > > trips {
> > > > cpu_alert6: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > > > @@ -1839,7 +1839,7 @@
> > > >
> > > > trips {
> > > > cpu_alert7: trip0 {
> > > > -   temperature = <75000>;
> > > > +   temperature = <95000>;
> > > > hysteresis = <2000>;
> > > > type = "passive";
> > > > };
> > >
> > > The change itself looks good to me, however I wonder if it would be
> > > worth to eliminate redundancy and merge the current 8 thermal zones
> > > into 2, one for the Silver and one for the Gold cluster (as done by
> > > http://crrev.com/c/1381752). There is a single cooling device for
> > > each cluster, so it's not clear to me if there is any gain from having
> > > a separate thermal zone for each CPU. If it is important to monitor
> > > the temperatures of the individual cores this can still be done by
> > > configuring the thermal zone of the cluster with multiple thermal
> > > sensors.
> >
> > I see your idea is to have a cooling device per CPU ("arm64: dts:
> > 

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Amit Kucheria]

On Thu, Jan 10, 2019 at 7:45 AM Matthias Kaehlcke  wrote:
>
> On Wed, Jan 09, 2019 at 05:15:33PM -0800, Matthias Kaehlcke wrote:
> > Hi Amit,
> >
> > On Thu, Jan 10, 2019 at 05:30:55AM +0530, Amit Kucheria wrote:
> > > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > > Qualcomm engineers, increase it to 95 degrees.
> > >
> > > Signed-off-by: Amit Kucheria 
> > > ---
> > >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > index c27cbd3bcb0a..29e823b0caf4 100644
> > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > @@ -1692,7 +1692,7 @@
> > >
> > > trips {
> > > cpu_alert0: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1713,7 +1713,7 @@
> > >
> > > trips {
> > > cpu_alert1: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1734,7 +1734,7 @@
> > >
> > > trips {
> > > cpu_alert2: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1755,7 +1755,7 @@
> > >
> > > trips {
> > > cpu_alert3: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1776,7 +1776,7 @@
> > >
> > > trips {
> > > cpu_alert4: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1797,7 +1797,7 @@
> > >
> > > trips {
> > > cpu_alert5: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1818,7 +1818,7 @@
> > >
> > > trips {
> > > cpu_alert6: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> > > @@ -1839,7 +1839,7 @@
> > >
> > > trips {
> > > cpu_alert7: trip0 {
> > > -   temperature = <75000>;
> > > +   temperature = <95000>;
> > > hysteresis = <2000>;
> > > type = "passive";
> > > };
> >
> > The change itself looks good to me, however I wonder if it would be
> > worth to eliminate redundancy and merge the current 8 thermal zones
> > into 2, one for the Silver and one for the Gold cluster (as done by
> > http://crrev.com/c/1381752). There is a single cooling device for
> > each cluster, so it's not clear to me if there is any gain from having
> > a separate thermal zone for each CPU. If it is important to monitor
> > the temperatures of the individual cores this can still be done by
> > configuring the thermal zone of the cluster with multiple thermal
> > sensors.
>
> I see your idea is to have a cooling device per CPU ("arm64: dts:
> sdm845: wireup the thermal trip points to cpufreq" /
> https://lore.kernel.org/patchwork/patch/1030742/), however that
> doesn't work as intended. Only two cpufreq 'devices' are created,
> one for CPU0 and one for CPU4. In consequence cpufreq->ready() only
> runs for 

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Doug Anderson]

Hi,

On Wed, Jan 9, 2019 at 4:29 PM Stephen Boyd  wrote:
>
> Quoting Amit Kucheria (2019-01-09 16:00:55)
> > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > Qualcomm engineers, increase it to 95 degrees.
> >
> > Signed-off-by: Amit Kucheria 
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
>
> Is the plan that these are some defaults that would be adjusted by board
> variants? Just curious why we have anything in here and don't punt it
> all to each board dts file.

My preference would be that the SoC device tree file should contain
thermal numbers that are important to pay attention to for the safety
/ proper operation of the SoC.  ...then individual boards could (if
they needed to) override with lower values to control, for instance,
skin temperature.

>From experience with previous boards, if you've got enough an off-SoC
thermistors then those are the ones you'd want to monitor to control
skin temperature.  It's OK if the SoC spikes up quite high as long as
that heat has somewhere to go (like a heat pipe).  The sensors that
are part of Amit's patch are on-chip.

...if you've got a board without external thermistors and are using
the SoC's on-chip sensors as a proxy for the heat in the overall
system then you might want to lower your values in the board device
tree file.  You won't be able to have as many short term spikes, but
that's what you gotta do without the extra sensors.


Sound sane?

-Doug

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Matthias Kaehlcke]

On Wed, Jan 09, 2019 at 05:15:33PM -0800, Matthias Kaehlcke wrote:
> Hi Amit,
> 
> On Thu, Jan 10, 2019 at 05:30:55AM +0530, Amit Kucheria wrote:
> > 75 degrees is too aggressive for throttling the CPU. After speaking to
> > Qualcomm engineers, increase it to 95 degrees.
> > 
> > Signed-off-by: Amit Kucheria 
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index c27cbd3bcb0a..29e823b0caf4 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -1692,7 +1692,7 @@
> >  
> > trips {
> > cpu_alert0: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1713,7 +1713,7 @@
> >  
> > trips {
> > cpu_alert1: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1734,7 +1734,7 @@
> >  
> > trips {
> > cpu_alert2: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1755,7 +1755,7 @@
> >  
> > trips {
> > cpu_alert3: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1776,7 +1776,7 @@
> >  
> > trips {
> > cpu_alert4: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1797,7 +1797,7 @@
> >  
> > trips {
> > cpu_alert5: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1818,7 +1818,7 @@
> >  
> > trips {
> > cpu_alert6: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> > @@ -1839,7 +1839,7 @@
> >  
> > trips {
> > cpu_alert7: trip0 {
> > -   temperature = <75000>;
> > +   temperature = <95000>;
> > hysteresis = <2000>;
> > type = "passive";
> > };
> 
> The change itself looks good to me, however I wonder if it would be
> worth to eliminate redundancy and merge the current 8 thermal zones
> into 2, one for the Silver and one for the Gold cluster (as done by
> http://crrev.com/c/1381752). There is a single cooling device for
> each cluster, so it's not clear to me if there is any gain from having
> a separate thermal zone for each CPU. If it is important to monitor
> the temperatures of the individual cores this can still be done by
> configuring the thermal zone of the cluster with multiple thermal
> sensors.

I see your idea is to have a cooling device per CPU ("arm64: dts:
sdm845: wireup the thermal trip points to cpufreq" /
https://lore.kernel.org/patchwork/patch/1030742/), however that
doesn't work as intended. Only two cpufreq 'devices' are created,
one for CPU0 and one for CPU4. In consequence cpufreq->ready() only
runs for these cores and only two cooling devices are
registered. Since the cores of a cluster all run at the same
frequency I also doubt if having multiple cooling devices would
bring any benefits.

Cheers

Matthias

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Matthias Kaehlcke]

Hi Amit,

On Thu, Jan 10, 2019 at 05:30:55AM +0530, Amit Kucheria wrote:
> 75 degrees is too aggressive for throttling the CPU. After speaking to
> Qualcomm engineers, increase it to 95 degrees.
> 
> Signed-off-by: Amit Kucheria 
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index c27cbd3bcb0a..29e823b0caf4 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1692,7 +1692,7 @@
>  
>   trips {
>   cpu_alert0: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1713,7 +1713,7 @@
>  
>   trips {
>   cpu_alert1: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1734,7 +1734,7 @@
>  
>   trips {
>   cpu_alert2: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1755,7 +1755,7 @@
>  
>   trips {
>   cpu_alert3: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1776,7 +1776,7 @@
>  
>   trips {
>   cpu_alert4: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1797,7 +1797,7 @@
>  
>   trips {
>   cpu_alert5: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1818,7 +1818,7 @@
>  
>   trips {
>   cpu_alert6: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };
> @@ -1839,7 +1839,7 @@
>  
>   trips {
>   cpu_alert7: trip0 {
> - temperature = <75000>;
> + temperature = <95000>;
>   hysteresis = <2000>;
>   type = "passive";
>   };

The change itself looks good to me, however I wonder if it would be
worth to eliminate redundancy and merge the current 8 thermal zones
into 2, one for the Silver and one for the Gold cluster (as done by
http://crrev.com/c/1381752). There is a single cooling device for
each cluster, so it's not clear to me if there is any gain from having
a separate thermal zone for each CPU. If it is important to monitor
the temperatures of the individual cores this can still be done by
configuring the thermal zone of the cluster with multiple thermal
sensors.

Cheers

Matthias

Re: [PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Stephen Boyd]

Quoting Amit Kucheria (2019-01-09 16:00:55)
> 75 degrees is too aggressive for throttling the CPU. After speaking to
> Qualcomm engineers, increase it to 95 degrees.
> 
> Signed-off-by: Amit Kucheria 
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 

Is the plan that these are some defaults that would be adjusted by board
variants? Just curious why we have anything in here and don't punt it
all to each board dts file.

[PATCH v1 6/7] arm64: dts: sdm845: Increase alert trip point to 95 degrees

[Amit Kucheria]

75 degrees is too aggressive for throttling the CPU. After speaking to
Qualcomm engineers, increase it to 95 degrees.

Signed-off-by: Amit Kucheria 
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index c27cbd3bcb0a..29e823b0caf4 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1692,7 +1692,7 @@
 
trips {
cpu_alert0: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1713,7 +1713,7 @@
 
trips {
cpu_alert1: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1734,7 +1734,7 @@
 
trips {
cpu_alert2: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1755,7 +1755,7 @@
 
trips {
cpu_alert3: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1776,7 +1776,7 @@
 
trips {
cpu_alert4: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1797,7 +1797,7 @@
 
trips {
cpu_alert5: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1818,7 +1818,7 @@
 
trips {
cpu_alert6: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
@@ -1839,7 +1839,7 @@
 
trips {
cpu_alert7: trip0 {
-   temperature = <75000>;
+   temperature = <95000>;
hysteresis = <2000>;
type = "passive";
};
-- 
2.17.1

Hacking Alert! You account was hacked (your password:qwerty)

[linux-kernel]

Dear user of vger.kernel.org!

I am a spyware software developer.
Your account has been hacked by me in the summer of 2018.

I understand that it is hard to believe, but here is my evidence:
- I sent you this email from your account.
- Password from account linux-kernel@vger.kernel.org: qwerty (on moment of 
hack).

The hacking was carried out using a hardware vulnerability through which you 
went online (Cisco router, vulnerability CVE-2018-0296).

I went around the security system in the router, installed an exploit there.
When you went online, my exploit downloaded my malicious code (rootkit) to your 
device.
This is driver software, I constantly updated it, so your antivirus is silent 
all time.

Since then I have been following you (I can connect to your device via the VNC 
protocol).
That is, I can see absolutely everything that you do, view and download your 
files and any data to yourself.
I also have access to the camera on your device, and I periodically take photos 
and videos with you.

At the moment, I have harvested a solid dirt... on you...
I saved all your email and chats from your messangers. I also saved the entire 
history of the sites you visit.

I note that it is useless to change the passwords. My malware update passwords 
from your accounts every times.

I know what you like hard funs (adult sites).
Oh, yes .. I'm know your secret life, which you are hiding from everyone.
Oh my God, what are your like... I saw THIS ... Oh, you dirty naughty person 
... :)

I took photos and videos of your most passionate funs with adult content, and 
synchronized them in real time with the image of your camera.
Believe it turned out very high quality!

So, to the business!
I'm sure you don't want to show these files and visiting history to all your 
contacts.

Transfer $868 to my Bitcoin cryptocurrency wallet: 
1Bt4psBJmjfVTcW6eYiJZ6HEbpFgKkBSX4
Just copy and paste the wallet number when transferring.
If you do not know how to do this - ask Google.

My system automatically recognizes the translation.
As soon as the specified amount is received, all your data will be destroyed 
from my server, and the rootkit will be automatically removed from your system.
Do not worry, I really will delete everything, since I am working 
with many people who have fallen into your position.
You will only have to inform your provider about the vulnerabilities in the 
router so that other hackers will not use it.

Since opening this letter you have 48 hours.
If funds not will be received, after the specified time has elapsed, the disk 
of your device will be formatted,
and from my server will automatically send email and sms to all your contacts 
with compromising material.

I advise you to remain prudent and not engage in nonsense (all files on my 
server).

Good luck!

Hacking Alert! You account was hacked (your password:qwerty)

[linux-kernel]

Dear user of vger.kernel.org!

I am a spyware software developer.
Your account has been hacked by me in the summer of 2018.

I understand that it is hard to believe, but here is my evidence:
- I sent you this email from your account.
- Password from account linux-kernel@vger.kernel.org: qwerty (on moment of 
hack).

The hacking was carried out using a hardware vulnerability through which you 
went online (Cisco router, vulnerability CVE-2018-0296).

I went around the security system in the router, installed an exploit there.
When you went online, my exploit downloaded my malicious code (rootkit) to your 
device.
This is driver software, I constantly updated it, so your antivirus is silent 
all time.

Since then I have been following you (I can connect to your device via the VNC 
protocol).
That is, I can see absolutely everything that you do, view and download your 
files and any data to yourself.
I also have access to the camera on your device, and I periodically take photos 
and videos with you.

At the moment, I have harvested a solid dirt... on you...
I saved all your email and chats from your messangers. I also saved the entire 
history of the sites you visit.

I note that it is useless to change the passwords. My malware update passwords 
from your accounts every times.

I know what you like hard funs (adult sites).
Oh, yes .. I'm know your secret life, which you are hiding from everyone.
Oh my God, what are your like... I saw THIS ... Oh, you dirty naughty person 
... :)

I took photos and videos of your most passionate funs with adult content, and 
synchronized them in real time with the image of your camera.
Believe it turned out very high quality!

So, to the business!
I'm sure you don't want to show these files and visiting history to all your 
contacts.

Transfer $868 to my Bitcoin cryptocurrency wallet: 
1Bt4psBJmjfVTcW6eYiJZ6HEbpFgKkBSX4
Just copy and paste the wallet number when transferring.
If you do not know how to do this - ask Google.

My system automatically recognizes the translation.
As soon as the specified amount is received, all your data will be destroyed 
from my server, and the rootkit will be automatically removed from your system.
Do not worry, I really will delete everything, since I am working 
with many people who have fallen into your position.
You will only have to inform your provider about the vulnerabilities in the 
router so that other hackers will not use it.

Since opening this letter you have 48 hours.
If funds not will be received, after the specified time has elapsed, the disk 
of your device will be formatted,
and from my server will automatically send email and sms to all your contacts 
with compromising material.

I advise you to remain prudent and not engage in nonsense (all files on my 
server).

Good luck!

Jackpot Email Alert-Only 9 Hours Left power-Ball Mega-Millions!!

[POWERBALL UNITED STATES]

POWERBALL HEADQUARTERS FLORIDA
250 Marriot main street
Dr Tallahassee 32301
President Home Supplies
100 Broadway Lane
United states of America NW80QE.
REF: SNT/FRN/17LL12-2018,

POWER-BALL EMAIL JACKPOT IS ON AGAIN!!!
THE USA POWERBALL MEGA MILLION JUST REACHED AN AMAZING JACKPOT OF USD 8.2M YOU 
HAVE 9 HOURS LEFT TO FILL THE SPACED COLUMNS,TIME IS RUNNING OUT TAKE A BOLD 
CHANCE TO BECOME A MILLONAIRE YOU CHOOSE TO BE. KINDLY FILL THE COLUMNS BELOW 
WITH YOUR CORRECT INFORMATION`S FOR YOUR AWARD WINNING CHEQUE TO BE FINALLY 
ENDORSED BY OUR PARTNER BANK(BANK OF AMERICA) IN OUR ONGOING POWERBALL EMAIL 
AWARDS JACKPOT LOTTERY 2018 ITS AMAZING HOW YOU WIN.YOUR EMAIL DRAW WINNING 
NUMBER: 291011/101992 AMOUNT: {$8.2,000,000.00 MILLION USD}

YOUR FULL HOME ADDRESS:
YOUR FULL NAMES:
NAME OF NEXT OF KIN:
PHONE NUMBER/FAX
OCCUPATION:
VALID ID CARD:
DATE OF BIRTH:
NATIONALITY:
GENDER:

GOOD LUCK,
COSTUMER SERVICE TEAM
POWERBALL UNITED STATES.

Contact the verification officer
for more information and guidelines.
email: dw5877...@gmail.com
©Copyright 2018 Publishers POWERBALL EMAIL LOTTERY. All Rights Reserved 

Jackpot Email Alert-Only 9 Hours Left power-Ball Mega-Millions!!

[POWERBALL UNITED STATES]

POWERBALL HEADQUARTERS FLORIDA
250 Marriot main street
Dr Tallahassee 32301
President Home Supplies
100 Broadway Lane
United states of America NW80QE.
REF: SNT/FRN/17LL12-2018,

POWER-BALL EMAIL JACKPOT IS ON AGAIN!!!
THE USA POWERBALL MEGA MILLION JUST REACHED AN AMAZING JACKPOT OF USD 8.2M YOU 
HAVE 9 HOURS LEFT TO FILL THE SPACED COLUMNS,TIME IS RUNNING OUT TAKE A BOLD 
CHANCE TO BECOME A MILLONAIRE YOU CHOOSE TO BE. KINDLY FILL THE COLUMNS BELOW 
WITH YOUR CORRECT INFORMATION`S FOR YOUR AWARD WINNING CHEQUE TO BE FINALLY 
ENDORSED BY OUR PARTNER BANK(BANK OF AMERICA) IN OUR ONGOING POWERBALL EMAIL 
AWARDS JACKPOT LOTTERY 2018 ITS AMAZING HOW YOU WIN.YOUR EMAIL DRAW WINNING 
NUMBER: 291011/101992 AMOUNT: {$8.2,000,000.00 MILLION USD}

YOUR FULL HOME ADDRESS:
YOUR FULL NAMES:
NAME OF NEXT OF KIN:
PHONE NUMBER/FAX
OCCUPATION:
VALID ID CARD:
DATE OF BIRTH:
NATIONALITY:
GENDER:

GOOD LUCK,
COSTUMER SERVICE TEAM
POWERBALL UNITED STATES.

Contact the verification officer
for more information and guidelines.
email: dw5877...@gmail.com
©Copyright 2018 Publishers POWERBALL EMAIL LOTTERY. All Rights Reserved 

Re: [PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Matheus Castello]

Hi Krzysztof and Sebastian,

please forgive me for the delay in working with this patch.
I've been having some personal issues these months, but leaving the 
excuses I still intend to send a v2 for this.


Thanks Krzysztof for review and tips I'll work on it.

Best Regards,
Matheus Castello

On 09/17/2018 08:32 AM, Krzysztof Kozlowski wrote:

On Sun, 16 Sep 2018 at 22:03, Sebastian Reichel
 wrote:


Hi Matheus,

Did I miss a v2 of this patchset, that solves the issues
found by Krzysztof?


I did not see v2. This patchset brings nice feature so it would be a
pity if it were discontinued.

Best regards,
Krzysztof

Re: [PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Matheus Castello]

Hi Krzysztof and Sebastian,

please forgive me for the delay in working with this patch.
I've been having some personal issues these months, but leaving the 
excuses I still intend to send a v2 for this.


Thanks Krzysztof for review and tips I'll work on it.

Best Regards,
Matheus Castello

On 09/17/2018 08:32 AM, Krzysztof Kozlowski wrote:

On Sun, 16 Sep 2018 at 22:03, Sebastian Reichel
 wrote:


Hi Matheus,

Did I miss a v2 of this patchset, that solves the issues
found by Krzysztof?


I did not see v2. This patchset brings nice feature so it would be a
pity if it were discontinued.

Best regards,
Krzysztof

Re: [PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Krzysztof Kozlowski]

On Sun, 16 Sep 2018 at 22:03, Sebastian Reichel
 wrote:
>
> Hi Matheus,
>
> Did I miss a v2 of this patchset, that solves the issues
> found by Krzysztof?

I did not see v2. This patchset brings nice feature so it would be a
pity if it were discontinued.

Best regards,
Krzysztof

Re: [PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Krzysztof Kozlowski]

On Sun, 16 Sep 2018 at 22:03, Sebastian Reichel
 wrote:
>
> Hi Matheus,
>
> Did I miss a v2 of this patchset, that solves the issues
> found by Krzysztof?

I did not see v2. This patchset brings nice feature so it would be a
pity if it were discontinued.

Best regards,
Krzysztof

Re: [PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Sebastian Reichel]

Hi Matheus,

Did I miss a v2 of this patchset, that solves the issues
found by Krzysztof?

-- Sebastian

On Mon, Jul 23, 2018 at 12:08:12AM -0400, Matheus Castello wrote:
> This series add IRQ handler for low level SOC alert, define a devicetree 
> binding attribute to configure the alert level threshold and check for changes
> in SOC for send uevents.
> 
> Max17040 have a pin for alert host about low level state of charge and this 
> alert can be configured in a threshold from 1% up to 32% of SOC.
> 
> Tested on Raspberry Pi Zero W, with a SparkFun Lipo Fuel Gauge module based on
> MAXIM MAX17043.
> 
> Matheus Castello (4):
>   power: supply: max17040: Add IRQ handler for low SOC alert
>   power: supply: max17040: Config alert SOC low level threshold from FDT
>   dt-bindings: power: supply: Max17040: Add low level SOC alert threshold
>   power: supply: max17040: Send uevent in SOC changes
> 
>  .../bindings/power/supply/max17040_battery.txt | 24 ++
>  drivers/power/supply/max17040_battery.c| 95 
> ++
>  2 files changed, 119 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> 
> -- 
> 2.13.3
> 


signature.asc
Description: PGP signature

Re: [PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Sebastian Reichel]

Hi Matheus,

Did I miss a v2 of this patchset, that solves the issues
found by Krzysztof?

-- Sebastian

On Mon, Jul 23, 2018 at 12:08:12AM -0400, Matheus Castello wrote:
> This series add IRQ handler for low level SOC alert, define a devicetree 
> binding attribute to configure the alert level threshold and check for changes
> in SOC for send uevents.
> 
> Max17040 have a pin for alert host about low level state of charge and this 
> alert can be configured in a threshold from 1% up to 32% of SOC.
> 
> Tested on Raspberry Pi Zero W, with a SparkFun Lipo Fuel Gauge module based on
> MAXIM MAX17043.
> 
> Matheus Castello (4):
>   power: supply: max17040: Add IRQ handler for low SOC alert
>   power: supply: max17040: Config alert SOC low level threshold from FDT
>   dt-bindings: power: supply: Max17040: Add low level SOC alert threshold
>   power: supply: max17040: Send uevent in SOC changes
> 
>  .../bindings/power/supply/max17040_battery.txt | 24 ++
>  drivers/power/supply/max17040_battery.c| 95 
> ++
>  2 files changed, 119 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> 
> -- 
> 2.13.3
> 


signature.asc
Description: PGP signature

Re: [PATCH 3/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On 23 July 2018 at 06:08, Matheus Castello  wrote:
> For configure low level state of charge threshold alert signaled from
> max17040 we add "maxim,alert-soc-level" property.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../bindings/power/supply/max17040_battery.txt | 24 
> ++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> new file mode 100644
> index ..e6e4e46c03c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -0,0 +1,24 @@
> +max17040_battery
> +

> +
> +Required properties :
> + - compatible : "maxim,max17040"

Why you skipped "maxim,max77836-battery"?

> +
> +Optional threshold properties :
> + If skipped the power up default value will be used
> + - maxim,alert-soc-level : The alert threshold that sets the state of
> +   charge level where an interrupt is generated.
> +   max17040 can be configured from 1 up to 32.
> +
> +Remembering that for the interrupt to be handled it must also be described 
> the
> +information of the interruption in the node.

Just mention interrupts in optional properties, including the flags.
BTW, the driver hard-codes the flags which is in contrast with DT
here.

Best regards,
Krzysztof

> +
> +Example:
> +
> +   battery-charger@36 {
> +   compatible = "maxim,max17040";
> +   reg = <0x36>;
> +   maxim,alert-soc-level = <10>;
> +   interrupt-parent = <>;
> +   interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +   };
> --
> 2.13.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On 23 July 2018 at 06:08, Matheus Castello  wrote:
> For configure low level state of charge threshold alert signaled from
> max17040 we add "maxim,alert-soc-level" property.
>
> Signed-off-by: Matheus Castello 
> ---
>  .../bindings/power/supply/max17040_battery.txt | 24 
> ++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
> b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> new file mode 100644
> index ..e6e4e46c03c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -0,0 +1,24 @@
> +max17040_battery
> +

> +
> +Required properties :
> + - compatible : "maxim,max17040"

Why you skipped "maxim,max77836-battery"?

> +
> +Optional threshold properties :
> + If skipped the power up default value will be used
> + - maxim,alert-soc-level : The alert threshold that sets the state of
> +   charge level where an interrupt is generated.
> +   max17040 can be configured from 1 up to 32.
> +
> +Remembering that for the interrupt to be handled it must also be described 
> the
> +information of the interruption in the node.

Just mention interrupts in optional properties, including the flags.
BTW, the driver hard-codes the flags which is in contrast with DT
here.

Best regards,
Krzysztof

> +
> +Example:
> +
> +   battery-charger@36 {
> +   compatible = "maxim,max17040";
> +   reg = <0x36>;
> +   maxim,alert-soc-level = <10>;
> +   interrupt-parent = <>;
> +   interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +   };
> --
> 2.13.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] power: supply: max17040: Config alert SOC low level threshold from FDT

[Krzysztof Kozlowski]

On 23 July 2018 at 06:08, Matheus Castello  wrote:
> For configuration of fuel gauge alert for a low level state of charge
> interrupt we add a function to config level threshold and a device tree
> binding property to set it in flatned device tree node.
>
> Now we can use "maxim,alert-soc-level" property with the values from
> 1 up to 32 to configure alert interrupt threshold.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 36 
> +
>  1 file changed, 36 insertions(+)

Hi Matheus,

In series, bindings describing new DT property should go before
introducing them in the driver.

>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index 6e54e58814a9..3efa52d32b44 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -47,6 +47,8 @@ struct max17040_chip {
> int soc;
> /* State Of Charge */
> int status;
> +   /* Alert threshold */

Threshold in what units?

> +   int alert_threshold;
>  };
>
>  static int max17040_get_property(struct power_supply *psy,
> @@ -123,6 +125,28 @@ static void max17040_get_soc(struct i2c_client *client)
> chip->soc = (soc >> 8);
>  }
>
> +static int max17040_set_soc_threshold(struct i2c_client *client, u8 level)
> +{
> +   int ret;
> +   u16 data;
> +
> +   /* check if level is between 1% and 32% */
> +   if (level > 0 && level < 33) {
> +   /* alert threshold use LSb 5 bits from RCOMP */
> +   /* two's-complements form: 0 = 32% and 1 = 1% */

Please use Linux style comments.

> +   level = 32 - level;
> +   data = max17040_read_reg(client, MAX17040_RCOMP);
> +   data &= 0xFFE0;

Please define the mask.

> +   data |= level;
> +   max17040_write_reg(client, MAX17040_RCOMP, data);
> +   ret = 0;
> +   } else {
> +   ret = -EINVAL;
> +   }
> +
> +   return ret;
> +}
> +
>  static void max17040_get_version(struct i2c_client *client)
>  {
> u16 version;
> @@ -165,6 +189,16 @@ static void max17040_get_status(struct i2c_client 
> *client)
> chip->status = POWER_SUPPLY_STATUS_FULL;
>  }
>
> +static void max17040_get_of_data(struct max17040_chip *chip)
> +{
> +   struct device *dev = >client->dev;
> +   struct device_node *np = dev->of_node;
> +
> +   if (of_property_read_s32(np, "maxim,alert-soc-level",
> +   >alert_threshold))
> +   chip->alert_threshold = 4;

1. You read s32 and later cast it to u8. Either some validation of
possible values or of_property_read_u8().
2. You have hard-coded default value - please put it in some define.
There are already defines, e.g.: MAX17040_BATTERY_FULL
3. Also the bindings say something about power up value... not 4.
4, I think that  default - missing - value should mean "no interrupt warnings".

> +}
> +
>  static void max17040_work(struct work_struct *work)
>  {
> struct max17040_chip *chip;
> @@ -231,6 +265,7 @@ static int max17040_probe(struct i2c_client *client,
>
> chip->client = client;
> chip->pdata = client->dev.platform_data;
> +   max17040_get_of_data(chip);
>
> i2c_set_clientdata(client, chip);
> psy_cfg.drv_data = chip;
> @@ -262,6 +297,7 @@ static int max17040_probe(struct i2c_client *client,
>
> max17040_reset(client);
> max17040_get_version(client);
> +   max17040_set_soc_threshold(client, chip->alert_threshold);

1. Return value ignored.
2. First you enable interrupts for whatever default value of alerts
and then you set the alerts threshold. You might generate false
warning in such case so this should be in reverse order.

Best regards,
Krzysztof

>
> INIT_DEFERRABLE_WORK(>work, max17040_work);
> queue_delayed_work(system_power_efficient_wq, >work,
> --
> 2.13.3
>

Re: [PATCH 2/4] power: supply: max17040: Config alert SOC low level threshold from FDT

[Krzysztof Kozlowski]

On 23 July 2018 at 06:08, Matheus Castello  wrote:
> For configuration of fuel gauge alert for a low level state of charge
> interrupt we add a function to config level threshold and a device tree
> binding property to set it in flatned device tree node.
>
> Now we can use "maxim,alert-soc-level" property with the values from
> 1 up to 32 to configure alert interrupt threshold.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 36 
> +
>  1 file changed, 36 insertions(+)

Hi Matheus,

In series, bindings describing new DT property should go before
introducing them in the driver.

>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index 6e54e58814a9..3efa52d32b44 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -47,6 +47,8 @@ struct max17040_chip {
> int soc;
> /* State Of Charge */
> int status;
> +   /* Alert threshold */

Threshold in what units?

> +   int alert_threshold;
>  };
>
>  static int max17040_get_property(struct power_supply *psy,
> @@ -123,6 +125,28 @@ static void max17040_get_soc(struct i2c_client *client)
> chip->soc = (soc >> 8);
>  }
>
> +static int max17040_set_soc_threshold(struct i2c_client *client, u8 level)
> +{
> +   int ret;
> +   u16 data;
> +
> +   /* check if level is between 1% and 32% */
> +   if (level > 0 && level < 33) {
> +   /* alert threshold use LSb 5 bits from RCOMP */
> +   /* two's-complements form: 0 = 32% and 1 = 1% */

Please use Linux style comments.

> +   level = 32 - level;
> +   data = max17040_read_reg(client, MAX17040_RCOMP);
> +   data &= 0xFFE0;

Please define the mask.

> +   data |= level;
> +   max17040_write_reg(client, MAX17040_RCOMP, data);
> +   ret = 0;
> +   } else {
> +   ret = -EINVAL;
> +   }
> +
> +   return ret;
> +}
> +
>  static void max17040_get_version(struct i2c_client *client)
>  {
> u16 version;
> @@ -165,6 +189,16 @@ static void max17040_get_status(struct i2c_client 
> *client)
> chip->status = POWER_SUPPLY_STATUS_FULL;
>  }
>
> +static void max17040_get_of_data(struct max17040_chip *chip)
> +{
> +   struct device *dev = >client->dev;
> +   struct device_node *np = dev->of_node;
> +
> +   if (of_property_read_s32(np, "maxim,alert-soc-level",
> +   >alert_threshold))
> +   chip->alert_threshold = 4;

1. You read s32 and later cast it to u8. Either some validation of
possible values or of_property_read_u8().
2. You have hard-coded default value - please put it in some define.
There are already defines, e.g.: MAX17040_BATTERY_FULL
3. Also the bindings say something about power up value... not 4.
4, I think that  default - missing - value should mean "no interrupt warnings".

> +}
> +
>  static void max17040_work(struct work_struct *work)
>  {
> struct max17040_chip *chip;
> @@ -231,6 +265,7 @@ static int max17040_probe(struct i2c_client *client,
>
> chip->client = client;
> chip->pdata = client->dev.platform_data;
> +   max17040_get_of_data(chip);
>
> i2c_set_clientdata(client, chip);
> psy_cfg.drv_data = chip;
> @@ -262,6 +297,7 @@ static int max17040_probe(struct i2c_client *client,
>
> max17040_reset(client);
> max17040_get_version(client);
> +   max17040_set_soc_threshold(client, chip->alert_threshold);

1. Return value ignored.
2. First you enable interrupts for whatever default value of alerts
and then you set the alerts threshold. You might generate false
warning in such case so this should be in reverse order.

Best regards,
Krzysztof

>
> INIT_DEFERRABLE_WORK(>work, max17040_work);
> queue_delayed_work(system_power_efficient_wq, >work,
> --
> 2.13.3
>

Re: [PATCH 1/4] power: supply: max17040: Add IRQ handler for low SOC alert

[Krzysztof Kozlowski]

On 23 July 2018 at 06:08, Matheus Castello  wrote:
> According datasheet max17040 has a pin for alert host for low SOC.
> This pin can be used as external interrupt, so we need to check for
> interrupts assigned for device and handle it.
>
> In hadler we are checking and storing fuel gauge registers values
> and send an uevent to notificate user space, so user space can decide
> save work or turn off since the alert demonstrate that the battery may
> no have the power to keep the system turned on for much longer.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 51 
> +
>  1 file changed, 51 insertions(+)
>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index 33c40f79d23d..6e54e58814a9 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -179,6 +180,24 @@ static void max17040_work(struct work_struct *work)
>MAX17040_DELAY);
>  }
>
> +static irqreturn_t max17040_thread_handler(int id, void *dev)
> +{
> +   struct max17040_chip *chip = dev;
> +   struct i2c_client *client = chip->client;
> +
> +   dev_warn(>dev, "IRQ: Alert battery low level");
> +   /* read registers */
> +   max17040_get_vcell(chip->client);

You just stored chip->client in client...

> +   max17040_get_soc(chip->client);
> +   max17040_get_online(chip->client);
> +   max17040_get_status(chip->client);

This duplicates max17040_work(). Can you split common part?

> +
> +   /* send uevent */
> +   power_supply_changed(chip->battery);
> +
> +   return IRQ_HANDLED;
> +}
> +
>  static enum power_supply_property max17040_battery_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> @@ -200,6 +219,8 @@ static int max17040_probe(struct i2c_client *client,
> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> struct power_supply_config psy_cfg = {};
> struct max17040_chip *chip;
> +   int ret;
> +   unsigned int flags;

Define them in scope using them.

>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
> return -EIO;
> @@ -221,6 +242,24 @@ static int max17040_probe(struct i2c_client *client,
> return PTR_ERR(chip->battery);
> }
>
> +   /* check interrupt */
> +   if (client->irq) {
> +   dev_info(>dev, "IRQ: enabled\n");
> +   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
> +
> +   ret = devm_request_threaded_irq(>dev, client->irq,
> +   NULL,
> +   max17040_thread_handler, 
> flags,
> +   chip->battery->desc->name,
> +   chip);

Please indent it to parenthesis.

> +   if (ret) {
> +   client->irq = 0;
> +   if (ret != -EBUSY)
> +   dev_warn(>dev,
> +   "Failed to get IRQ err %d \n", ret);
> +   }
> +   }
> +
> max17040_reset(client);
> max17040_get_version(client);
>
> @@ -248,6 +287,12 @@ static int max17040_suspend(struct device *dev)
> struct max17040_chip *chip = i2c_get_clientdata(client);
>
> cancel_delayed_work(>work);
> +
> +   if (chip->client->irq) {

I think this should use device wakeup properties (e.g.
device_init_wakeup(), device_may_wakeup()) coming from pdata.

It would be nice to CC users of this driver. We have it on some of
boards. Unfortunately get_maintainer will not point it automatically
so:
scripts/get_maintainer.pl -f drivers/mfd/max14577.c

Best regards,
Krzysztof

> +   disable_irq(chip->client->irq);
> +   enable_irq_wake(chip->client->irq);
> +   }
> +
> return 0;
>  }
>
> @@ -258,6 +303,12 @@ static int max17040_resume(struct device *dev)
>
> queue_delayed_work(system_power_efficient_wq, >work,
>MAX17040_DELAY);
> +
> +   if (chip->client->irq) {
> +   disable_irq_wake(chip->client->irq);
> +   enable_irq(chip->client->irq);
> +   }
> +
> return 0;
>  }
>
> --
> 2.13.3
>

Re: [PATCH 1/4] power: supply: max17040: Add IRQ handler for low SOC alert

[Krzysztof Kozlowski]

On 23 July 2018 at 06:08, Matheus Castello  wrote:
> According datasheet max17040 has a pin for alert host for low SOC.
> This pin can be used as external interrupt, so we need to check for
> interrupts assigned for device and handle it.
>
> In hadler we are checking and storing fuel gauge registers values
> and send an uevent to notificate user space, so user space can decide
> save work or turn off since the alert demonstrate that the battery may
> no have the power to keep the system turned on for much longer.
>
> Signed-off-by: Matheus Castello 
> ---
>  drivers/power/supply/max17040_battery.c | 51 
> +
>  1 file changed, 51 insertions(+)
>
> diff --git a/drivers/power/supply/max17040_battery.c 
> b/drivers/power/supply/max17040_battery.c
> index 33c40f79d23d..6e54e58814a9 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -179,6 +180,24 @@ static void max17040_work(struct work_struct *work)
>MAX17040_DELAY);
>  }
>
> +static irqreturn_t max17040_thread_handler(int id, void *dev)
> +{
> +   struct max17040_chip *chip = dev;
> +   struct i2c_client *client = chip->client;
> +
> +   dev_warn(>dev, "IRQ: Alert battery low level");
> +   /* read registers */
> +   max17040_get_vcell(chip->client);

You just stored chip->client in client...

> +   max17040_get_soc(chip->client);
> +   max17040_get_online(chip->client);
> +   max17040_get_status(chip->client);

This duplicates max17040_work(). Can you split common part?

> +
> +   /* send uevent */
> +   power_supply_changed(chip->battery);
> +
> +   return IRQ_HANDLED;
> +}
> +
>  static enum power_supply_property max17040_battery_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> @@ -200,6 +219,8 @@ static int max17040_probe(struct i2c_client *client,
> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> struct power_supply_config psy_cfg = {};
> struct max17040_chip *chip;
> +   int ret;
> +   unsigned int flags;

Define them in scope using them.

>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
> return -EIO;
> @@ -221,6 +242,24 @@ static int max17040_probe(struct i2c_client *client,
> return PTR_ERR(chip->battery);
> }
>
> +   /* check interrupt */
> +   if (client->irq) {
> +   dev_info(>dev, "IRQ: enabled\n");
> +   flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
> +
> +   ret = devm_request_threaded_irq(>dev, client->irq,
> +   NULL,
> +   max17040_thread_handler, 
> flags,
> +   chip->battery->desc->name,
> +   chip);

Please indent it to parenthesis.

> +   if (ret) {
> +   client->irq = 0;
> +   if (ret != -EBUSY)
> +   dev_warn(>dev,
> +   "Failed to get IRQ err %d \n", ret);
> +   }
> +   }
> +
> max17040_reset(client);
> max17040_get_version(client);
>
> @@ -248,6 +287,12 @@ static int max17040_suspend(struct device *dev)
> struct max17040_chip *chip = i2c_get_clientdata(client);
>
> cancel_delayed_work(>work);
> +
> +   if (chip->client->irq) {

I think this should use device wakeup properties (e.g.
device_init_wakeup(), device_may_wakeup()) coming from pdata.

It would be nice to CC users of this driver. We have it on some of
boards. Unfortunately get_maintainer will not point it automatically
so:
scripts/get_maintainer.pl -f drivers/mfd/max14577.c

Best regards,
Krzysztof

> +   disable_irq(chip->client->irq);
> +   enable_irq_wake(chip->client->irq);
> +   }
> +
> return 0;
>  }
>
> @@ -258,6 +303,12 @@ static int max17040_resume(struct device *dev)
>
> queue_delayed_work(system_power_efficient_wq, >work,
>MAX17040_DELAY);
> +
> +   if (chip->client->irq) {
> +   disable_irq_wake(chip->client->irq);
> +   enable_irq(chip->client->irq);
> +   }
> +
> return 0;
>  }
>
> --
> 2.13.3
>

[PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Matheus Castello]

This series add IRQ handler for low level SOC alert, define a devicetree 
binding attribute to configure the alert level threshold and check for changes
in SOC for send uevents.

Max17040 have a pin for alert host about low level state of charge and this 
alert can be configured in a threshold from 1% up to 32% of SOC.

Tested on Raspberry Pi Zero W, with a SparkFun Lipo Fuel Gauge module based on
MAXIM MAX17043.

Matheus Castello (4):
  power: supply: max17040: Add IRQ handler for low SOC alert
  power: supply: max17040: Config alert SOC low level threshold from FDT
  dt-bindings: power: supply: Max17040: Add low level SOC alert threshold
  power: supply: max17040: Send uevent in SOC changes

 .../bindings/power/supply/max17040_battery.txt | 24 ++
 drivers/power/supply/max17040_battery.c| 95 ++
 2 files changed, 119 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

-- 
2.13.3

[PATCH 0/4] power: supply: MAX17040: Add IRQ for low level and alert SOC changes

[Matheus Castello]

This series add IRQ handler for low level SOC alert, define a devicetree 
binding attribute to configure the alert level threshold and check for changes
in SOC for send uevents.

Max17040 have a pin for alert host about low level state of charge and this 
alert can be configured in a threshold from 1% up to 32% of SOC.

Tested on Raspberry Pi Zero W, with a SparkFun Lipo Fuel Gauge module based on
MAXIM MAX17043.

Matheus Castello (4):
  power: supply: max17040: Add IRQ handler for low SOC alert
  power: supply: max17040: Config alert SOC low level threshold from FDT
  dt-bindings: power: supply: Max17040: Add low level SOC alert threshold
  power: supply: max17040: Send uevent in SOC changes

 .../bindings/power/supply/max17040_battery.txt | 24 ++
 drivers/power/supply/max17040_battery.c| 95 ++
 2 files changed, 119 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

-- 
2.13.3

[PATCH 3/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Matheus Castello]

For configure low level state of charge threshold alert signaled from
max17040 we add "maxim,alert-soc-level" property.

Signed-off-by: Matheus Castello 
---
 .../bindings/power/supply/max17040_battery.txt | 24 ++
 1 file changed, 24 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

diff --git 
a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index ..e6e4e46c03c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,24 @@
+max17040_battery
+
+
+Required properties :
+ - compatible : "maxim,max17040"
+
+Optional threshold properties :
+ If skipped the power up default value will be used
+ - maxim,alert-soc-level : The alert threshold that sets the state of
+   charge level where an interrupt is generated.
+   max17040 can be configured from 1 up to 32.
+
+Remembering that for the interrupt to be handled it must also be described the
+information of the interruption in the node.
+
+Example:
+
+   battery-charger@36 {
+   compatible = "maxim,max17040";
+   reg = <0x36>;
+   maxim,alert-soc-level = <10>;
+   interrupt-parent = <>;
+   interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+   };
-- 
2.13.3

[PATCH 3/4] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Matheus Castello]

For configure low level state of charge threshold alert signaled from
max17040 we add "maxim,alert-soc-level" property.

Signed-off-by: Matheus Castello 
---
 .../bindings/power/supply/max17040_battery.txt | 24 ++
 1 file changed, 24 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/supply/max17040_battery.txt

diff --git 
a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt 
b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
new file mode 100644
index ..e6e4e46c03c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -0,0 +1,24 @@
+max17040_battery
+
+
+Required properties :
+ - compatible : "maxim,max17040"
+
+Optional threshold properties :
+ If skipped the power up default value will be used
+ - maxim,alert-soc-level : The alert threshold that sets the state of
+   charge level where an interrupt is generated.
+   max17040 can be configured from 1 up to 32.
+
+Remembering that for the interrupt to be handled it must also be described the
+information of the interruption in the node.
+
+Example:
+
+   battery-charger@36 {
+   compatible = "maxim,max17040";
+   reg = <0x36>;
+   maxim,alert-soc-level = <10>;
+   interrupt-parent = <>;
+   interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+   };
-- 
2.13.3

[PATCH 2/4] power: supply: max17040: Config alert SOC low level threshold from FDT

[Matheus Castello]

For configuration of fuel gauge alert for a low level state of charge
interrupt we add a function to config level threshold and a device tree
binding property to set it in flatned device tree node.

Now we can use "maxim,alert-soc-level" property with the values from
1 up to 32 to configure alert interrupt threshold.

Signed-off-by: Matheus Castello 
---
 drivers/power/supply/max17040_battery.c | 36 +
 1 file changed, 36 insertions(+)

diff --git a/drivers/power/supply/max17040_battery.c 
b/drivers/power/supply/max17040_battery.c
index 6e54e58814a9..3efa52d32b44 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -47,6 +47,8 @@ struct max17040_chip {
int soc;
/* State Of Charge */
int status;
+   /* Alert threshold */
+   int alert_threshold;
 };
 
 static int max17040_get_property(struct power_supply *psy,
@@ -123,6 +125,28 @@ static void max17040_get_soc(struct i2c_client *client)
chip->soc = (soc >> 8);
 }
 
+static int max17040_set_soc_threshold(struct i2c_client *client, u8 level)
+{
+   int ret;
+   u16 data;
+
+   /* check if level is between 1% and 32% */
+   if (level > 0 && level < 33) {
+   /* alert threshold use LSb 5 bits from RCOMP */
+   /* two's-complements form: 0 = 32% and 1 = 1% */
+   level = 32 - level;
+   data = max17040_read_reg(client, MAX17040_RCOMP);
+   data &= 0xFFE0;
+   data |= level;
+   max17040_write_reg(client, MAX17040_RCOMP, data);
+   ret = 0;
+   } else {
+   ret = -EINVAL;
+   }
+
+   return ret;
+}
+
 static void max17040_get_version(struct i2c_client *client)
 {
u16 version;
@@ -165,6 +189,16 @@ static void max17040_get_status(struct i2c_client *client)
chip->status = POWER_SUPPLY_STATUS_FULL;
 }
 
+static void max17040_get_of_data(struct max17040_chip *chip)
+{
+   struct device *dev = >client->dev;
+   struct device_node *np = dev->of_node;
+
+   if (of_property_read_s32(np, "maxim,alert-soc-level",
+   >alert_threshold))
+   chip->alert_threshold = 4;
+}
+
 static void max17040_work(struct work_struct *work)
 {
struct max17040_chip *chip;
@@ -231,6 +265,7 @@ static int max17040_probe(struct i2c_client *client,
 
chip->client = client;
chip->pdata = client->dev.platform_data;
+   max17040_get_of_data(chip);
 
i2c_set_clientdata(client, chip);
psy_cfg.drv_data = chip;
@@ -262,6 +297,7 @@ static int max17040_probe(struct i2c_client *client,
 
max17040_reset(client);
max17040_get_version(client);
+   max17040_set_soc_threshold(client, chip->alert_threshold);
 
INIT_DEFERRABLE_WORK(>work, max17040_work);
queue_delayed_work(system_power_efficient_wq, >work,
-- 
2.13.3