Thanks Damien for your comments here. Will make these changes in
PATCH V2.
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On 3/28/22 17:42, Ajish Koshy wrote:
> > When upper inbound and outbound queues 32-63 are enabled, we see
> upper
> > vectors 32-63 in interrupt service routine. We need corresponding
> > registers to handle masking and unmasking of these upper interrupts.
> >
> > To achieve this, we use registers MSGU_ODMR_U(0x34) to mask and
> > MSGU_ODMR_CLR_U(0x3C) to unmask the interrupts. In these registers bit
> > 0-31 represents interrupt vectors 32-63.
> >
> > Signed-off-by: Ajish Koshy <Ajish.Koshy@xxxxxxxxxxxxx>
> > Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> > ---
> > drivers/scsi/pm8001/pm80xx_hwi.c | 35
> > +++++++++++++++++++++++++++-----
> > 1 file changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c
> > b/drivers/scsi/pm8001/pm80xx_hwi.c
> > index 9bb31f66db85..b92e82a576e3 100644
> > --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> > @@ -1728,9 +1728,20 @@ pm80xx_chip_interrupt_enable(struct
> > pm8001_hba_info *pm8001_ha, u8 vec) { #ifdef PM8001_USE_MSIX
> > u32 mask;
> > - mask = (u32)(1 << vec);
> > + u32 vec_u;
> >
> > - pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR, (u32)(mask &
> 0xFFFFFFFF));
> > + if (vec < 32) {
> > + mask = (u32)(1 << vec);
> > + /*vectors 0 - 31*/
> > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR,
> > + (u32)(mask & 0xFFFFFFFF));
> > + } else {
> > + vec_u = vec - 32;
> > + mask = (u32)(1 << vec_u);
> > + /*vectors 32 - 63*/
> > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_CLR_U,
> > + (u32)(mask & 0xFFFFFFFF));
> > + }
> > return;
> > #endif
> > pm80xx_chip_intx_interrupt_enable(pm8001_ha);
> > @@ -1747,11 +1758,25 @@ pm80xx_chip_interrupt_disable(struct
> > pm8001_hba_info *pm8001_ha, u8 vec) { #ifdef PM8001_USE_MSIX
> > u32 mask;
> > - if (vec == 0xFF)
> > + u32 vec_u;
> > +
> > + if (vec == 0xFF) {
> > mask = 0xFFFFFFFF;
> > - else
> > + /* disable all vectors 0-31, 32-63*/
> > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, mask);
> > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, mask);
> > + } else if (vec < 32) {
> > mask = (u32)(1 << vec);
> > - pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, (u32)(mask &
> 0xFFFFFFFF));
> > + /*vectors 0 - 31*/
> > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR,
> > + (u32)(mask & 0xFFFFFFFF));
>
> mask is a u32, so the "& 0xFFFFFFFF" seems to be completely pointless...
> And pm8001_cw32() takes a u32 value as last argument so the cast is also
> pointless.
OK
>
> > + } else {
> > + vec_u = vec - 32;
> > + mask = (u32)(1 << vec_u);
>
> Cast not needed and this should probably be "1U << vec_u".
> Also, the vec_u variable seems completely unnecessary.
OK
>
> > + /*vectors 32 - 63*/
> > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U,
> > + (u32)(mask & 0xFFFFFFFF));
>
> Same here: The cast and masking are not needed.
OK
>
> > + }
> > return;
> > #endif
> > pm80xx_chip_intx_interrupt_disable(pm8001_ha);
>
>
> --
> Damien Le Moal
> Western Digital Research
