* [PATCH 0/2] pm80xx updates @ 2022-03-28 8:42 Ajish Koshy 2022-03-28 8:42 ` [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 Ajish Koshy 2022-03-28 8:42 ` [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues Ajish Koshy 0 siblings, 2 replies; 13+ messages in thread From: Ajish Koshy @ 2022-03-28 8:42 UTC (permalink / raw) To: linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry, Jinpu Wang This patchset includes bugfixes for pm80xx driver Ajish Koshy (2): scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 scsi: pm80xx: enable upper inbound, outbound queues drivers/scsi/pm8001/pm80xx_hwi.c | 42 ++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 2022-03-28 8:42 [PATCH 0/2] pm80xx updates Ajish Koshy @ 2022-03-28 8:42 ` Ajish Koshy 2022-03-28 9:20 ` Damien Le Moal 2022-03-28 9:39 ` Jinpu Wang 2022-03-28 8:42 ` [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues Ajish Koshy 1 sibling, 2 replies; 13+ messages in thread From: Ajish Koshy @ 2022-03-28 8:42 UTC (permalink / raw) To: linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry, Jinpu Wang 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@microchip.com> Signed-off-by: Viswas G <Viswas.G@microchip.com> --- 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)); + } else { + vec_u = vec - 32; + mask = (u32)(1 << vec_u); + /*vectors 32 - 63*/ + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, + (u32)(mask & 0xFFFFFFFF)); + } return; #endif pm80xx_chip_intx_interrupt_disable(pm8001_ha); -- 2.31.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 2022-03-28 8:42 ` [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 Ajish Koshy @ 2022-03-28 9:20 ` Damien Le Moal 2022-03-28 10:12 ` Ajish.Koshy 2022-03-28 9:39 ` Jinpu Wang 1 sibling, 1 reply; 13+ messages in thread From: Damien Le Moal @ 2022-03-28 9:20 UTC (permalink / raw) To: Ajish Koshy, linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, john.garry, Jinpu Wang 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@microchip.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> > --- > 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. > + } 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. > + /*vectors 32 - 63*/ > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, > + (u32)(mask & 0xFFFFFFFF)); Same here: The cast and masking are not needed. > + } > return; > #endif > pm80xx_chip_intx_interrupt_disable(pm8001_ha); -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 2022-03-28 9:20 ` Damien Le Moal @ 2022-03-28 10:12 ` Ajish.Koshy 0 siblings, 0 replies; 13+ messages in thread From: Ajish.Koshy @ 2022-03-28 10:12 UTC (permalink / raw) To: damien.lemoal, linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, john.garry, jinpu.wang 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@microchip.com> > > Signed-off-by: Viswas G <Viswas.G@microchip.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 2022-03-28 8:42 ` [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 Ajish Koshy 2022-03-28 9:20 ` Damien Le Moal @ 2022-03-28 9:39 ` Jinpu Wang 2022-03-28 10:53 ` Ajish.Koshy 1 sibling, 1 reply; 13+ messages in thread From: Jinpu Wang @ 2022-03-28 9:39 UTC (permalink / raw) To: Ajish Koshy Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry, Jinpu Wang On Mon, Mar 28, 2022 at 10:42 AM Ajish Koshy <Ajish.Koshy@microchip.com> 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@microchip.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> same as patch2, the fixes commit should be add here. > --- > 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)); > + } else { > + vec_u = vec - 32; > + mask = (u32)(1 << vec_u); > + /*vectors 32 - 63*/ > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, > + (u32)(mask & 0xFFFFFFFF)); > + } > return; > #endif > pm80xx_chip_intx_interrupt_disable(pm8001_ha); > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 2022-03-28 9:39 ` Jinpu Wang @ 2022-03-28 10:53 ` Ajish.Koshy 0 siblings, 0 replies; 13+ messages in thread From: Ajish.Koshy @ 2022-03-28 10:53 UTC (permalink / raw) To: jinpu.wang Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry Thanks Jinpu. > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Mon, Mar 28, 2022 at 10:42 AM Ajish Koshy > <Ajish.Koshy@microchip.com> 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@microchip.com> > > Signed-off-by: Viswas G <Viswas.G@microchip.com> > same as patch2, the fixes commit should be add here. Will make the changes in V2. > > --- > > 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)); > > + } else { > > + vec_u = vec - 32; > > + mask = (u32)(1 << vec_u); > > + /*vectors 32 - 63*/ > > + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR_U, > > + (u32)(mask & 0xFFFFFFFF)); > > + } > > return; > > #endif > > pm80xx_chip_intx_interrupt_disable(pm8001_ha); > > -- > > 2.31.1 > > Thanks, Ajish ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 8:42 [PATCH 0/2] pm80xx updates Ajish Koshy 2022-03-28 8:42 ` [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 Ajish Koshy @ 2022-03-28 8:42 ` Ajish Koshy 2022-03-28 9:23 ` Damien Le Moal ` (2 more replies) 1 sibling, 3 replies; 13+ messages in thread From: Ajish Koshy @ 2022-03-28 8:42 UTC (permalink / raw) To: linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry, Jinpu Wang Executing driver on servers with more than 32 CPUs were faced with command timeouts. This is because we were not geting completions for commands submitted on IQ32 - IQ63. Set E64Q bit to enable upper inbound and outbound queues 32 to 63 in the MPI main configuration table. Added 500ms delay after successful MPI initialization as mentioned in controller datasheet. Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com> Signed-off-by: Viswas G <Viswas.G@microchip.com> --- drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index b92e82a576e3..f04c6c589615 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -766,6 +766,10 @@ static void init_default_table_values(struct pm8001_hba_info *pm8001_ha) pm8001_ha->main_cfg_tbl.pm80xx_tbl.pcs_event_log_severity = 0x01; pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt = 0x01; + /* Enable higher IQs and OQs, 32 to 63, bit 16*/ + if (pm8001_ha->max_q_num > 32) + pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt |= + (1 << 16); /* Disable end to end CRC checking */ pm8001_ha->main_cfg_tbl.pm80xx_tbl.crc_core_dump = (0x1 << 16); @@ -1027,6 +1031,9 @@ static int mpi_init_check(struct pm8001_hba_info *pm8001_ha) if (0x0000 != gst_len_mpistate) return -EBUSY; + /* Wait for 500ms after successful MPI initialization*/ + msleep(500); + return 0; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 8:42 ` [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues Ajish Koshy @ 2022-03-28 9:23 ` Damien Le Moal 2022-03-28 10:48 ` Ajish.Koshy 2022-03-28 9:37 ` Jinpu Wang 2022-03-28 14:13 ` John Garry 2 siblings, 1 reply; 13+ messages in thread From: Damien Le Moal @ 2022-03-28 9:23 UTC (permalink / raw) To: Ajish Koshy, linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, john.garry, Jinpu Wang On 3/28/22 17:42, Ajish Koshy wrote: > Executing driver on servers with more than 32 CPUs were faced with command > timeouts. This is because we were not geting completions for commands > submitted on IQ32 - IQ63. > > Set E64Q bit to enable upper inbound and outbound queues 32 to 63 in the > MPI main configuration table. > > Added 500ms delay after successful MPI initialization as mentioned in > controller datasheet. > > Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> > --- > drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c > index b92e82a576e3..f04c6c589615 100644 > --- a/drivers/scsi/pm8001/pm80xx_hwi.c > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c > @@ -766,6 +766,10 @@ static void init_default_table_values(struct pm8001_hba_info *pm8001_ha) > pm8001_ha->main_cfg_tbl.pm80xx_tbl.pcs_event_log_severity = 0x01; > pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt = 0x01; > > + /* Enable higher IQs and OQs, 32 to 63, bit 16*/ > + if (pm8001_ha->max_q_num > 32) > + pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt |= > + (1 << 16); No need for the brackets. > /* Disable end to end CRC checking */ > pm8001_ha->main_cfg_tbl.pm80xx_tbl.crc_core_dump = (0x1 << 16); > > @@ -1027,6 +1031,9 @@ static int mpi_init_check(struct pm8001_hba_info *pm8001_ha) > if (0x0000 != gst_len_mpistate) > return -EBUSY; > > + /* Wait for 500ms after successful MPI initialization*/ Is this comment really necessary ? Anybody sees that this will wait. It may be better to explain *why* the wait is needed. > + msleep(500); > + > return 0; > } > -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 9:23 ` Damien Le Moal @ 2022-03-28 10:48 ` Ajish.Koshy 0 siblings, 0 replies; 13+ messages in thread From: Ajish.Koshy @ 2022-03-28 10:48 UTC (permalink / raw) To: damien.lemoal, linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, john.garry, jinpu.wang 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: > > Executing driver on servers with more than 32 CPUs were faced with > > command timeouts. This is because we were not geting completions for > > commands submitted on IQ32 - IQ63. > > > > Set E64Q bit to enable upper inbound and outbound queues 32 to 63 in > > the MPI main configuration table. > > > > Added 500ms delay after successful MPI initialization as mentioned in > > controller datasheet. > > > > Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com> > > Signed-off-by: Viswas G <Viswas.G@microchip.com> > > --- > > drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c > > b/drivers/scsi/pm8001/pm80xx_hwi.c > > index b92e82a576e3..f04c6c589615 100644 > > --- a/drivers/scsi/pm8001/pm80xx_hwi.c > > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c > > @@ -766,6 +766,10 @@ static void init_default_table_values(struct > pm8001_hba_info *pm8001_ha) > > pm8001_ha->main_cfg_tbl.pm80xx_tbl.pcs_event_log_severity = > 0x01; > > pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt = 0x01; > > > > + /* Enable higher IQs and OQs, 32 to 63, bit 16*/ > > + if (pm8001_ha->max_q_num > 32) > > + pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt |= > > + (1 << 16); > > No need for the brackets. OK > > > /* Disable end to end CRC checking */ > > pm8001_ha->main_cfg_tbl.pm80xx_tbl.crc_core_dump = (0x1 << 16); > > > > @@ -1027,6 +1031,9 @@ static int mpi_init_check(struct > pm8001_hba_info *pm8001_ha) > > if (0x0000 != gst_len_mpistate) > > return -EBUSY; > > > > + /* Wait for 500ms after successful MPI initialization*/ > > Is this comment really necessary ? Anybody sees that this will wait. It may be > better to explain *why* the wait is needed. Yes you may be right here. The code itself speaks about this. The reason for delay is not mentioned in the datasheet. When going through this E64Q bit details they mentioned about the MPI initialization steps to be followed. Since MPI Initialization is mentioned separately irrespective of E64Q bit, I believe this may be needed to be followed in all the cases. Well here controller datasheet is not talking much on this wait. It simply says "Note: It is recommended to wait 500ms after the MPI-S field indicates the host has successfully initialized the MPI before sending commands." > > > + msleep(500); > > + > > return 0; > > } > > > > > -- > Damien Le Moal > Western Digital Research Thanks, Ajish ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 8:42 ` [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues Ajish Koshy 2022-03-28 9:23 ` Damien Le Moal @ 2022-03-28 9:37 ` Jinpu Wang 2022-03-28 10:51 ` Ajish.Koshy 2022-03-28 14:13 ` John Garry 2 siblings, 1 reply; 13+ messages in thread From: Jinpu Wang @ 2022-03-28 9:37 UTC (permalink / raw) To: Ajish Koshy Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry, Jinpu Wang On Mon, Mar 28, 2022 at 10:42 AM Ajish Koshy <Ajish.Koshy@microchip.com> wrote: > > Executing driver on servers with more than 32 CPUs were faced with command > timeouts. This is because we were not geting completions for commands > submitted on IQ32 - IQ63. > > Set E64Q bit to enable upper inbound and outbound queues 32 to 63 in the > MPI main configuration table. > > Added 500ms delay after successful MPI initialization as mentioned in > controller datasheet. > > Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> Please add Fixes: 05c6c029a44d ("scsi: pm80xx: Increase number of supported queues") so it will pickup automatically by stable. > --- > drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c > index b92e82a576e3..f04c6c589615 100644 > --- a/drivers/scsi/pm8001/pm80xx_hwi.c > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c > @@ -766,6 +766,10 @@ static void init_default_table_values(struct pm8001_hba_info *pm8001_ha) > pm8001_ha->main_cfg_tbl.pm80xx_tbl.pcs_event_log_severity = 0x01; > pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt = 0x01; > > + /* Enable higher IQs and OQs, 32 to 63, bit 16*/ > + if (pm8001_ha->max_q_num > 32) > + pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt |= > + (1 << 16); > /* Disable end to end CRC checking */ > pm8001_ha->main_cfg_tbl.pm80xx_tbl.crc_core_dump = (0x1 << 16); > > @@ -1027,6 +1031,9 @@ static int mpi_init_check(struct pm8001_hba_info *pm8001_ha) > if (0x0000 != gst_len_mpistate) > return -EBUSY; > > + /* Wait for 500ms after successful MPI initialization*/ > + msleep(500); > + > return 0; > } > > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 9:37 ` Jinpu Wang @ 2022-03-28 10:51 ` Ajish.Koshy 0 siblings, 0 replies; 13+ messages in thread From: Ajish.Koshy @ 2022-03-28 10:51 UTC (permalink / raw) To: jinpu.wang Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, john.garry Thanks Jinpu for your comments here. Will make the changes in PATCH V2. > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Mon, Mar 28, 2022 at 10:42 AM Ajish Koshy > <Ajish.Koshy@microchip.com> wrote: > > > > Executing driver on servers with more than 32 CPUs were faced with > > command timeouts. This is because we were not geting completions for > > commands submitted on IQ32 - IQ63. > > > > Set E64Q bit to enable upper inbound and outbound queues 32 to 63 in > > the MPI main configuration table. > > > > Added 500ms delay after successful MPI initialization as mentioned in > > controller datasheet. > > > > Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com> > > Signed-off-by: Viswas G <Viswas.G@microchip.com> > Please add > Fixes: 05c6c029a44d ("scsi: pm80xx: Increase number of supported > queues") so it will pickup > automatically by stable. OK > > > --- > > drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c > > b/drivers/scsi/pm8001/pm80xx_hwi.c > > index b92e82a576e3..f04c6c589615 100644 > > --- a/drivers/scsi/pm8001/pm80xx_hwi.c > > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c > > @@ -766,6 +766,10 @@ static void init_default_table_values(struct > pm8001_hba_info *pm8001_ha) > > pm8001_ha->main_cfg_tbl.pm80xx_tbl.pcs_event_log_severity = > 0x01; > > pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt = > 0x01; > > > > + /* Enable higher IQs and OQs, 32 to 63, bit 16*/ > > + if (pm8001_ha->max_q_num > 32) > > + pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt |= > > + (1 << 16); > > /* Disable end to end CRC checking */ > > pm8001_ha->main_cfg_tbl.pm80xx_tbl.crc_core_dump = (0x1 << > > 16); > > > > @@ -1027,6 +1031,9 @@ static int mpi_init_check(struct > pm8001_hba_info *pm8001_ha) > > if (0x0000 != gst_len_mpistate) > > return -EBUSY; > > > > + /* Wait for 500ms after successful MPI initialization*/ > > + msleep(500); > > + > > return 0; > > } > > > > -- > > 2.31.1 > > Thanks, Ajish ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 8:42 ` [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues Ajish Koshy 2022-03-28 9:23 ` Damien Le Moal 2022-03-28 9:37 ` Jinpu Wang @ 2022-03-28 14:13 ` John Garry 2022-04-01 9:58 ` Ajish.Koshy 2 siblings, 1 reply; 13+ messages in thread From: John Garry @ 2022-03-28 14:13 UTC (permalink / raw) To: Ajish Koshy, linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, Jinpu Wang On 28/03/2022 09:42, Ajish Koshy wrote: > Executing driver on servers with more than 32 CPUs were faced with command > timeouts. This is because we were not geting completions for commands > submitted on IQ32 - IQ63. > > Set E64Q bit to enable upper inbound and outbound queues 32 to 63 in the > MPI main configuration table. > > Added 500ms delay after successful MPI initialization as mentioned in > controller datasheet. > > Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> > --- I'm not sure if this change was meant to help, but I still see timeouts on my 96-core arm64 machine with this change - see log at bottom. I still get the feeling that this issue I mention is timing related, as it goes away when I enable lots of heavy debug (like kasan, kmemleak, prove_locking, etc. > drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c > index b92e82a576e3..f04c6c589615 100644 > --- a/drivers/scsi/pm8001/pm80xx_hwi.c > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c > @@ -766,6 +766,10 @@ static void init_default_table_values(struct pm8001_hba_info *pm8001_ha) > pm8001_ha->main_cfg_tbl.pm80xx_tbl.pcs_event_log_severity = 0x01; > pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt = 0x01; > > + /* Enable higher IQs and OQs, 32 to 63, bit 16*/ > + if (pm8001_ha->max_q_num > 32) > + pm8001_ha->main_cfg_tbl.pm80xx_tbl.fatal_err_interrupt |= > + (1 << 16); > /* Disable end to end CRC checking */ > pm8001_ha->main_cfg_tbl.pm80xx_tbl.crc_core_dump = (0x1 << 16); > > @@ -1027,6 +1031,9 @@ static int mpi_init_check(struct pm8001_hba_info *pm8001_ha) > if (0x0000 != gst_len_mpistate) > return -EBUSY; > > + /* Wait for 500ms after successful MPI initialization*/ > + msleep(500); > + > return 0; > } > 126.037932] EXT4-fs (sda1): recovery complete [ 126.042297] EXT4-fs (sda1): mounted filesystem with ordered data mode. Quota mode: none. [ 159.939179] sas: Enter sas_scsi_recover_host busy: 256 failed: 256 [ 159.945390] sas: sas_scsi_find_task: aborting task 0x(____ptrval____) [ 181.862870] sas: TMF task timeout for 5000c500a7b95a49 and not done [ 193.436187] pm80xx0:: pm8001_abort_task 1126: rc= 5 [ 193.436188] pm80xx0:: mpi_ssp_completion 1937: sas IO status 0x1 [ 193.441064] sas: sas_scsi_find_task: querying task 0x(____ptrval____) [ 193.447048] pm80xx0:: mpi_ssp_completion 1948: SAS Address of IO Failure Drive:5000c500a7b95a49 [ 193.453528] pm80xx0:: mpi_ssp_completion 1937: sas IO status 0x3b [ 193.462158] pm80xx0:: mpi_ssp_completion 2185: task 0x(____ptrval____) done with io_status 0x1 resp 0x0 stat 0x8c but aborted by upper layer! [ 193.468237] pm80xx0:: mpi_ssp_completion 1948: SAS Address of IO Failure Drive:5000c500a7b95a49 [ 193.489658] sas: TMF task open reject failed 5000c500a7b95a49 [ 193.495538] pm80xx0:: mpi_ssp_completion 1937: sas IO status 0x3b [ 193.501619] pm80xx0:: mpi_ssp_completion 1948: SAS Address of IO Failure Drive:5000c500a7b95a49 [ 193.510371] sas: TMF task open reject failed 5000c500a7b95a49 [ 193.516252] pm80xx0:: mpi_ssp_completion 1937: sas IO status 0x3b [ 193.522333] pm80xx0:: mpi_ssp_completion 1948: SAS Address of IO Failure Drive:5000c500a7b95a49 [ 193.531075] sas: TMF task open reject failed 5000c500a7b95a49 [ 193.536899] sas: executing TMF for 5000c500a7b95a49 failed after 3 attempts! [ 193.543937] pm80xx: rc= -5 [ 193.546631] sas: sas_scsi_find_task: task 0x(____ptrval____) result code -5 not handled [ 193.554622] sas: sas_scsi_find_task: aborting task 0x(____ptrval____) [ 193.561052] sas: sas_eh_handle_sas_errors: task 0x(____ptrval____) is done [ 193.567917] sas: sas_scsi_find_task: aborting task 0x(____ptrval____) ls'ing 0 [ 214.630859] sas: TMF task timeout for 5000c500a7b95a49 and not done [ 226.241090] pm80xx0:: mpi_ssp_completion 1937: sas IO status 0x1 [ 226.241093] pm80xx0:: pm8001_abort_task 1126: rc= 5 [ 226.247084] pm80xx0:: mpi_ssp_completion 1948: SAS Address of IO Failure Drive:5000c500a7b95a49 [ 226.247087] pm80xx0:: mpi_ssp_completion 2185: task 0x(____ptrval____) done with io_status 0x1 resp 0x0 stat 0x8c but aborted by upper layer! [ 226.273324] sas: sas_eh_handle_sas_errors: task 0x(____ptrval____) is done [ 226.280188] sas: sas_scsi_find_task: aborting task 0x(____ptrval____) [ 247.398856] sas: TMF task timeout for 5000c500a7b95a49 and not done ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues 2022-03-28 14:13 ` John Garry @ 2022-04-01 9:58 ` Ajish.Koshy 0 siblings, 0 replies; 13+ messages in thread From: Ajish.Koshy @ 2022-04-01 9:58 UTC (permalink / raw) To: john.garry, linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, damien.lemoal, jinpu.wang Hi John, > I'm not sure if this change was meant to help, but I still see timeouts on my > 96-core arm64 machine with this change - see log at bottom. > > I still get the feeling that this issue I mention is timing related, as it goes away > when I enable lots of heavy debug (like kasan, kmemleak, prove_locking, etc. > Thank you for sharing your observation on arm server with respect to these patches. It will be difficult to comment right now on arm server and as you said issue is not observed when debugs are enabled. Current patch was tested on amd 128 core server x86_64, drives connected to enclosure. Loading driver(without patch) on this 128 core server had issues. To get better understanding, booted the server in two scenarios, boot arguments 1. nr_cpus=32 2. nr_cpus=34 For #1 nr_cpus=32, was able to load the driver, run fio, unload the driver without any issue. But for #2 nr_cpus=34, things were not working properly. Loading had issues. Error handler was getting triggered. After giving some debug prints, it was visible that request submitted on IQ31 or less were getting completion whereas for requests on IQ32 and IQ33 were not receiving completion/interrupt. Post enabling this E64Q bit in the controller, made a difference. Now driver can be loaded, run FIO, unload it without observing any issue, including on 128 core default setting. Thanks, Ajish ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-04-01 9:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-28 8:42 [PATCH 0/2] pm80xx updates Ajish Koshy 2022-03-28 8:42 ` [PATCH 1/2] scsi: pm80xx: mask and unmask upper interrupt vectors 32-63 Ajish Koshy 2022-03-28 9:20 ` Damien Le Moal 2022-03-28 10:12 ` Ajish.Koshy 2022-03-28 9:39 ` Jinpu Wang 2022-03-28 10:53 ` Ajish.Koshy 2022-03-28 8:42 ` [PATCH 2/2] scsi: pm80xx: enable upper inbound, outbound queues Ajish Koshy 2022-03-28 9:23 ` Damien Le Moal 2022-03-28 10:48 ` Ajish.Koshy 2022-03-28 9:37 ` Jinpu Wang 2022-03-28 10:51 ` Ajish.Koshy 2022-03-28 14:13 ` John Garry 2022-04-01 9:58 ` Ajish.Koshy
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.