All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
@ 2018-05-21 12:39 avifishman70
  2018-05-21 12:39 ` [PATCH v2 1/1] " avifishman70
  0 siblings, 1 reply; 7+ messages in thread
From: avifishman70 @ 2018-05-21 12:39 UTC (permalink / raw)
  To: minyard, arnd, gregkh, openipmi-developer, joel, openbmc
  Cc: haiyue.wang, tmaimon77, tali.perry1, avifishman70, Avi Fishman

From: Avi Fishman <AviFishman70@gmail.com>

Changes since version 1:
Apply code style comments from "Wang, Haiyue" <haiyue.wang@linux.intel.com>


Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
host on writes to output buffer.
This patch fixes it by setting the bits that enables the generation of
IRQn events by hardware control based on the status of the OBF flag.


Avi Fishman (1):
  ipmi: NPCM7xx KCS BMC: enable interrupt to the host

 drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
  2018-05-21 12:39 [PATCH v2 0/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host avifishman70
@ 2018-05-21 12:39 ` avifishman70
  2018-05-22 18:52   ` Corey Minyard
  0 siblings, 1 reply; 7+ messages in thread
From: avifishman70 @ 2018-05-21 12:39 UTC (permalink / raw)
  To: minyard, arnd, gregkh, openipmi-developer, joel, openbmc
  Cc: haiyue.wang, tmaimon77, tali.perry1, avifishman70, Avi Fishman

From: Avi Fishman <AviFishman70@gmail.com>

Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
host on writes to output buffer.
This patch fixes it by setting the bits that enables the generation of
IRQn events by hardware control based on the status of the OBF flag.

Signed-off-by: Avi Fishman <AviFishman70@gmail.com>
Reviewed-by:   Haiyue Wang <haiyue.wang@linux.intel.com>
---
 drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
index 7bc898c5d372..722f7391fe1f 100644
--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
@@ -39,6 +39,12 @@
 #define KCS3CTL		0x3C
 #define    KCS_CTL_IBFIE	BIT(0)
 
+#define KCS1IE		0x1C
+#define KCS2IE		0x2E
+#define KCS3IE		0x40
+#define    KCS_IE_IRQE          BIT(0)
+#define    KCS_IE_HIRQE         BIT(3)
+
 /*
  * 7.2.4 Core KCS Registers
  * Registers in this module are 8 bits. An 8-bit register must be accessed
@@ -48,12 +54,14 @@
  * dob: KCS Channel n Data Out Buffer Register (KCSnDO).
  * dib: KCS Channel n Data In Buffer Register (KCSnDI).
  * ctl: KCS Channel n Control Register (KCSnCTL).
+ * ie : KCS Channel n  Interrupt Enable Register (KCSnIE).
  */
 struct npcm7xx_kcs_reg {
 	u32 sts;
 	u32 dob;
 	u32 dib;
 	u32 ctl;
+	u32 ie;
 };
 
 struct npcm7xx_kcs_bmc {
@@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc {
 };
 
 static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
-	{ .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL },
-	{ .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL },
-	{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL },
+	{ .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL, .ie = KCS1IE },
+	{ .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL, .ie = KCS2IE },
+	{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE },
 };
 
 static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
@@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
 
 	regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
 			   enable ? KCS_CTL_IBFIE : 0);
+
+	regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE | KCS_IE_HIRQE,
+			   enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
 }
 
 static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
  2018-05-21 12:39 ` [PATCH v2 1/1] " avifishman70
@ 2018-05-22 18:52   ` Corey Minyard
  2018-05-23  9:40     ` Avi Fishman
  0 siblings, 1 reply; 7+ messages in thread
From: Corey Minyard @ 2018-05-22 18:52 UTC (permalink / raw)
  To: avifishman70, arnd, gregkh, openipmi-developer, joel, openbmc
  Cc: haiyue.wang, tmaimon77, tali.perry1

On 05/21/2018 07:39 AM, avifishman70@gmail.com wrote:
> From: Avi Fishman <AviFishman70@gmail.com>
>
> Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
> host on writes to output buffer.
> This patch fixes it by setting the bits that enables the generation of
> IRQn events by hardware control based on the status of the OBF flag.

This look ok, I've added it to my 4.19 queue.

-corey

> Signed-off-by: Avi Fishman <AviFishman70@gmail.com>
> Reviewed-by:   Haiyue Wang <haiyue.wang@linux.intel.com>
> ---
>   drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
> index 7bc898c5d372..722f7391fe1f 100644
> --- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
> +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
> @@ -39,6 +39,12 @@
>   #define KCS3CTL		0x3C
>   #define    KCS_CTL_IBFIE	BIT(0)
>   
> +#define KCS1IE		0x1C
> +#define KCS2IE		0x2E
> +#define KCS3IE		0x40
> +#define    KCS_IE_IRQE          BIT(0)
> +#define    KCS_IE_HIRQE         BIT(3)
> +
>   /*
>    * 7.2.4 Core KCS Registers
>    * Registers in this module are 8 bits. An 8-bit register must be accessed
> @@ -48,12 +54,14 @@
>    * dob: KCS Channel n Data Out Buffer Register (KCSnDO).
>    * dib: KCS Channel n Data In Buffer Register (KCSnDI).
>    * ctl: KCS Channel n Control Register (KCSnCTL).
> + * ie : KCS Channel n  Interrupt Enable Register (KCSnIE).
>    */
>   struct npcm7xx_kcs_reg {
>   	u32 sts;
>   	u32 dob;
>   	u32 dib;
>   	u32 ctl;
> +	u32 ie;
>   };
>   
>   struct npcm7xx_kcs_bmc {
> @@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc {
>   };
>   
>   static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
> -	{ .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL },
> -	{ .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL },
> -	{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL },
> +	{ .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL, .ie = KCS1IE },
> +	{ .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL, .ie = KCS2IE },
> +	{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE },
>   };
>   
>   static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
> @@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
>   
>   	regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>   			   enable ? KCS_CTL_IBFIE : 0);
> +
> +	regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE | KCS_IE_HIRQE,
> +			   enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
>   }
>   
>   static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
  2018-05-22 18:52   ` Corey Minyard
@ 2018-05-23  9:40     ` Avi Fishman
  2018-05-23 12:05       ` Corey Minyard
  0 siblings, 1 reply; 7+ messages in thread
From: Avi Fishman @ 2018-05-23  9:40 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Arnd Bergmann, Greg Kroah-Hartman, openipmi-developer,
	Joel Stanley, OpenBMC Maillist, Wang, Haiyue, Tomer Maimon,
	tali.perry1

On Tue, May 22, 2018 at 9:52 PM, Corey Minyard <minyard@acm.org> wrote:
> On 05/21/2018 07:39 AM, avifishman70@gmail.com wrote:
>>
>> From: Avi Fishman <AviFishman70@gmail.com>
>>
>> Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
>> host on writes to output buffer.
>> This patch fixes it by setting the bits that enables the generation of
>> IRQn events by hardware control based on the status of the OBF flag.
>
>
> This look ok, I've added it to my 4.19 queue.

Thanks Corey,

Since drivers/char/ipmi/kcs_bmc_npcm7xx.c already exists in linux-next
(4.18) what not adding it as a fix in 4.18?

-Avi

>
> -corey
>
>
>> Signed-off-by: Avi Fishman <AviFishman70@gmail.com>
>> Reviewed-by:   Haiyue Wang <haiyue.wang@linux.intel.com>
>> ---
>>   drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
>>   1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>> b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>> index 7bc898c5d372..722f7391fe1f 100644
>> --- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>> +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>> @@ -39,6 +39,12 @@
>>   #define KCS3CTL               0x3C
>>   #define    KCS_CTL_IBFIE      BIT(0)
>>   +#define KCS1IE               0x1C
>> +#define KCS2IE         0x2E
>> +#define KCS3IE         0x40
>> +#define    KCS_IE_IRQE          BIT(0)
>> +#define    KCS_IE_HIRQE         BIT(3)
>> +
>>   /*
>>    * 7.2.4 Core KCS Registers
>>    * Registers in this module are 8 bits. An 8-bit register must be
>> accessed
>> @@ -48,12 +54,14 @@
>>    * dob: KCS Channel n Data Out Buffer Register (KCSnDO).
>>    * dib: KCS Channel n Data In Buffer Register (KCSnDI).
>>    * ctl: KCS Channel n Control Register (KCSnCTL).
>> + * ie : KCS Channel n  Interrupt Enable Register (KCSnIE).
>>    */
>>   struct npcm7xx_kcs_reg {
>>         u32 sts;
>>         u32 dob;
>>         u32 dib;
>>         u32 ctl;
>> +       u32 ie;
>>   };
>>     struct npcm7xx_kcs_bmc {
>> @@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc {
>>   };
>>     static const struct npcm7xx_kcs_reg
>> npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
>> -       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL },
>> -       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL },
>> -       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL },
>> +       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL, .ie
>> = KCS1IE },
>> +       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL, .ie
>> = KCS2IE },
>> +       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie
>> = KCS3IE },
>>   };
>>     static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
>> @@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc
>> *kcs_bmc, bool enable)
>>         regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>>                            enable ? KCS_CTL_IBFIE : 0);
>> +
>> +       regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE |
>> KCS_IE_HIRQE,
>> +                          enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
>>   }
>>     static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
  2018-05-23  9:40     ` Avi Fishman
@ 2018-05-23 12:05       ` Corey Minyard
  2018-05-23 13:06         ` Avi Fishman
  0 siblings, 1 reply; 7+ messages in thread
From: Corey Minyard @ 2018-05-23 12:05 UTC (permalink / raw)
  To: Avi Fishman
  Cc: Arnd Bergmann, Greg Kroah-Hartman, openipmi-developer,
	Joel Stanley, OpenBMC Maillist, Wang, Haiyue, Tomer Maimon,
	tali.perry1

On 05/23/2018 04:40 AM, Avi Fishman wrote:
> On Tue, May 22, 2018 at 9:52 PM, Corey Minyard <minyard@acm.org> wrote:
>> On 05/21/2018 07:39 AM, avifishman70@gmail.com wrote:
>>> From: Avi Fishman <AviFishman70@gmail.com>
>>>
>>> Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
>>> host on writes to output buffer.
>>> This patch fixes it by setting the bits that enables the generation of
>>> IRQn events by hardware control based on the status of the OBF flag.
>>
>> This look ok, I've added it to my 4.19 queue.
> Thanks Corey,
>
> Since drivers/char/ipmi/kcs_bmc_npcm7xx.c already exists in linux-next
> (4.18) what not adding it as a fix in 4.18?

I try not to add new stuff at this stage in the game.  I was debating 
whether this is a bugfix or a new feature.  If you have a pressing need 
to have it in 4.18, I can consider that.

-corey

> -Avi
>
>> -corey
>>
>>
>>> Signed-off-by: Avi Fishman <AviFishman70@gmail.com>
>>> Reviewed-by:   Haiyue Wang <haiyue.wang@linux.intel.com>
>>> ---
>>>    drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
>>>    1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>> b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>> index 7bc898c5d372..722f7391fe1f 100644
>>> --- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>> +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>> @@ -39,6 +39,12 @@
>>>    #define KCS3CTL               0x3C
>>>    #define    KCS_CTL_IBFIE      BIT(0)
>>>    +#define KCS1IE               0x1C
>>> +#define KCS2IE         0x2E
>>> +#define KCS3IE         0x40
>>> +#define    KCS_IE_IRQE          BIT(0)
>>> +#define    KCS_IE_HIRQE         BIT(3)
>>> +
>>>    /*
>>>     * 7.2.4 Core KCS Registers
>>>     * Registers in this module are 8 bits. An 8-bit register must be
>>> accessed
>>> @@ -48,12 +54,14 @@
>>>     * dob: KCS Channel n Data Out Buffer Register (KCSnDO).
>>>     * dib: KCS Channel n Data In Buffer Register (KCSnDI).
>>>     * ctl: KCS Channel n Control Register (KCSnCTL).
>>> + * ie : KCS Channel n  Interrupt Enable Register (KCSnIE).
>>>     */
>>>    struct npcm7xx_kcs_reg {
>>>          u32 sts;
>>>          u32 dob;
>>>          u32 dib;
>>>          u32 ctl;
>>> +       u32 ie;
>>>    };
>>>      struct npcm7xx_kcs_bmc {
>>> @@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc {
>>>    };
>>>      static const struct npcm7xx_kcs_reg
>>> npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
>>> -       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL },
>>> -       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL },
>>> -       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL },
>>> +       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL, .ie
>>> = KCS1IE },
>>> +       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL, .ie
>>> = KCS2IE },
>>> +       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie
>>> = KCS3IE },
>>>    };
>>>      static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
>>> @@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc
>>> *kcs_bmc, bool enable)
>>>          regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>>>                             enable ? KCS_CTL_IBFIE : 0);
>>> +
>>> +       regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE |
>>> KCS_IE_HIRQE,
>>> +                          enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
>>>    }
>>>      static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
>>
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
  2018-05-23 12:05       ` Corey Minyard
@ 2018-05-23 13:06         ` Avi Fishman
  2018-05-23 13:30           ` Corey Minyard
  0 siblings, 1 reply; 7+ messages in thread
From: Avi Fishman @ 2018-05-23 13:06 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Arnd Bergmann, Greg Kroah-Hartman, openipmi-developer,
	Joel Stanley, OpenBMC Maillist, Wang, Haiyue, Tomer Maimon,
	Tali Perry

On Wed, May 23, 2018 at 3:05 PM, Corey Minyard <minyard@acm.org> wrote:
> On 05/23/2018 04:40 AM, Avi Fishman wrote:
>>
>> On Tue, May 22, 2018 at 9:52 PM, Corey Minyard <minyard@acm.org> wrote:
>>>
>>> On 05/21/2018 07:39 AM, avifishman70@gmail.com wrote:
>>>>
>>>> From: Avi Fishman <AviFishman70@gmail.com>
>>>>
>>>> Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
>>>> host on writes to output buffer.
>>>> This patch fixes it by setting the bits that enables the generation of
>>>> IRQn events by hardware control based on the status of the OBF flag.
>>>
>>>
>>> This look ok, I've added it to my 4.19 queue.
>>
>> Thanks Corey,
>>
>> Since drivers/char/ipmi/kcs_bmc_npcm7xx.c already exists in linux-next
>> (4.18) what not adding it as a fix in 4.18?
>
>
> I try not to add new stuff at this stage in the game.  I was debating
> whether this is a bugfix or a new feature.  If you have a pressing need to
> have it in 4.18, I can consider that.
>
> -corey

Without this fix the host is doing polling at a very slow rate, when
the host is triggered by an interrupt the response is much more
faster.
Note that the same people who tested and approved the original driver
reported the performance issue and they tested it with this fix and
also approved.
They and I will appriciate if it will be in 4.18 :)

-Avi

>
>
>> -Avi
>>
>>> -corey
>>>
>>>
>>>> Signed-off-by: Avi Fishman <AviFishman70@gmail.com>
>>>> Reviewed-by:   Haiyue Wang <haiyue.wang@linux.intel.com>
>>>> ---
>>>>    drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
>>>>    1 file changed, 14 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>> b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>> index 7bc898c5d372..722f7391fe1f 100644
>>>> --- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>> +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>> @@ -39,6 +39,12 @@
>>>>    #define KCS3CTL               0x3C
>>>>    #define    KCS_CTL_IBFIE      BIT(0)
>>>>    +#define KCS1IE               0x1C
>>>> +#define KCS2IE         0x2E
>>>> +#define KCS3IE         0x40
>>>> +#define    KCS_IE_IRQE          BIT(0)
>>>> +#define    KCS_IE_HIRQE         BIT(3)
>>>> +
>>>>    /*
>>>>     * 7.2.4 Core KCS Registers
>>>>     * Registers in this module are 8 bits. An 8-bit register must be
>>>> accessed
>>>> @@ -48,12 +54,14 @@
>>>>     * dob: KCS Channel n Data Out Buffer Register (KCSnDO).
>>>>     * dib: KCS Channel n Data In Buffer Register (KCSnDI).
>>>>     * ctl: KCS Channel n Control Register (KCSnCTL).
>>>> + * ie : KCS Channel n  Interrupt Enable Register (KCSnIE).
>>>>     */
>>>>    struct npcm7xx_kcs_reg {
>>>>          u32 sts;
>>>>          u32 dob;
>>>>          u32 dib;
>>>>          u32 ctl;
>>>> +       u32 ie;
>>>>    };
>>>>      struct npcm7xx_kcs_bmc {
>>>> @@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc {
>>>>    };
>>>>      static const struct npcm7xx_kcs_reg
>>>> npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
>>>> -       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL },
>>>> -       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL },
>>>> -       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL },
>>>> +       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL,
>>>> .ie
>>>> = KCS1IE },
>>>> +       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL,
>>>> .ie
>>>> = KCS2IE },
>>>> +       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL,
>>>> .ie
>>>> = KCS3IE },
>>>>    };
>>>>      static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
>>>> @@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc
>>>> *kcs_bmc, bool enable)
>>>>          regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>>>>                             enable ? KCS_CTL_IBFIE : 0);
>>>> +
>>>> +       regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE |
>>>> KCS_IE_HIRQE,
>>>> +                          enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
>>>>    }
>>>>      static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
>>>
>>>
>>>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host
  2018-05-23 13:06         ` Avi Fishman
@ 2018-05-23 13:30           ` Corey Minyard
  0 siblings, 0 replies; 7+ messages in thread
From: Corey Minyard @ 2018-05-23 13:30 UTC (permalink / raw)
  To: Avi Fishman
  Cc: Arnd Bergmann, Greg Kroah-Hartman, openipmi-developer,
	Joel Stanley, OpenBMC Maillist, Wang, Haiyue, Tomer Maimon,
	Tali Perry

On 05/23/2018 08:06 AM, Avi Fishman wrote:
> On Wed, May 23, 2018 at 3:05 PM, Corey Minyard <minyard@acm.org> wrote:
>> On 05/23/2018 04:40 AM, Avi Fishman wrote:
>>> On Tue, May 22, 2018 at 9:52 PM, Corey Minyard <minyard@acm.org> wrote:
>>>> On 05/21/2018 07:39 AM, avifishman70@gmail.com wrote:
>>>>> From: Avi Fishman <AviFishman70@gmail.com>
>>>>>
>>>>> Original kcs_bmc_npcm7xx.c was missing enabling to send interrupt to the
>>>>> host on writes to output buffer.
>>>>> This patch fixes it by setting the bits that enables the generation of
>>>>> IRQn events by hardware control based on the status of the OBF flag.
>>>>
>>>> This look ok, I've added it to my 4.19 queue.
>>> Thanks Corey,
>>>
>>> Since drivers/char/ipmi/kcs_bmc_npcm7xx.c already exists in linux-next
>>> (4.18) what not adding it as a fix in 4.18?
>>
>> I try not to add new stuff at this stage in the game.  I was debating
>> whether this is a bugfix or a new feature.  If you have a pressing need to
>> have it in 4.18, I can consider that.
>>
>> -corey
> Without this fix the host is doing polling at a very slow rate, when
> the host is triggered by an interrupt the response is much more
> faster.
> Note that the same people who tested and approved the original driver
> reported the performance issue and they tested it with this fix and
> also approved.
> They and I will appriciate if it will be in 4.18 :)
>
> -Avi

Ok, it's in my linux-next queue for 4.18.

-corey

>>
>>> -Avi
>>>
>>>> -corey
>>>>
>>>>
>>>>> Signed-off-by: Avi Fishman <AviFishman70@gmail.com>
>>>>> Reviewed-by:   Haiyue Wang <haiyue.wang@linux.intel.com>
>>>>> ---
>>>>>     drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++++++++++---
>>>>>     1 file changed, 14 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>>> b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>>> index 7bc898c5d372..722f7391fe1f 100644
>>>>> --- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>>> +++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>>>>> @@ -39,6 +39,12 @@
>>>>>     #define KCS3CTL               0x3C
>>>>>     #define    KCS_CTL_IBFIE      BIT(0)
>>>>>     +#define KCS1IE               0x1C
>>>>> +#define KCS2IE         0x2E
>>>>> +#define KCS3IE         0x40
>>>>> +#define    KCS_IE_IRQE          BIT(0)
>>>>> +#define    KCS_IE_HIRQE         BIT(3)
>>>>> +
>>>>>     /*
>>>>>      * 7.2.4 Core KCS Registers
>>>>>      * Registers in this module are 8 bits. An 8-bit register must be
>>>>> accessed
>>>>> @@ -48,12 +54,14 @@
>>>>>      * dob: KCS Channel n Data Out Buffer Register (KCSnDO).
>>>>>      * dib: KCS Channel n Data In Buffer Register (KCSnDI).
>>>>>      * ctl: KCS Channel n Control Register (KCSnCTL).
>>>>> + * ie : KCS Channel n  Interrupt Enable Register (KCSnIE).
>>>>>      */
>>>>>     struct npcm7xx_kcs_reg {
>>>>>           u32 sts;
>>>>>           u32 dob;
>>>>>           u32 dib;
>>>>>           u32 ctl;
>>>>> +       u32 ie;
>>>>>     };
>>>>>       struct npcm7xx_kcs_bmc {
>>>>> @@ -63,9 +71,9 @@ struct npcm7xx_kcs_bmc {
>>>>>     };
>>>>>       static const struct npcm7xx_kcs_reg
>>>>> npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
>>>>> -       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL },
>>>>> -       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL },
>>>>> -       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL },
>>>>> +       { .sts = KCS1ST, .dob = KCS1DO, .dib = KCS1DI, .ctl = KCS1CTL,
>>>>> .ie
>>>>> = KCS1IE },
>>>>> +       { .sts = KCS2ST, .dob = KCS2DO, .dib = KCS2DI, .ctl = KCS2CTL,
>>>>> .ie
>>>>> = KCS2IE },
>>>>> +       { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL,
>>>>> .ie
>>>>> = KCS3IE },
>>>>>     };
>>>>>       static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
>>>>> @@ -95,6 +103,9 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc
>>>>> *kcs_bmc, bool enable)
>>>>>           regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>>>>>                              enable ? KCS_CTL_IBFIE : 0);
>>>>> +
>>>>> +       regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE |
>>>>> KCS_IE_HIRQE,
>>>>> +                          enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
>>>>>     }
>>>>>       static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
>>>>
>>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-05-23 13:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-21 12:39 [PATCH v2 0/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host avifishman70
2018-05-21 12:39 ` [PATCH v2 1/1] " avifishman70
2018-05-22 18:52   ` Corey Minyard
2018-05-23  9:40     ` Avi Fishman
2018-05-23 12:05       ` Corey Minyard
2018-05-23 13:06         ` Avi Fishman
2018-05-23 13:30           ` Corey Minyard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.