[PATCH v2 1/1] ipmi: NPCM7xx KCS BMC: enable interrupt to the host

[Corey Minyard]

On 05/23/2018 08:06 AM, Avi Fishman wrote:
> On Wed, May 23, 2018 at 3:05 PM, Corey Minyard <minyard at 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 at acm.org> wrote:
>>>> On 05/21/2018 07:39 AM, avifishman70 at gmail.com wrote:
>>>>> From: Avi Fishman <AviFishman70 at 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 at gmail.com>
>>>>> Reviewed-by:   Haiyue Wang <haiyue.wang at 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)
>>>>
>>>>