Hey, > On 21. Aug 2020, at 20:00, Himanshu Madhani <himanshu.madhani@xxxxxxxxxx> wrote: > > > >> On Aug 21, 2020, at 12:49 PM, René Rebe <rene@xxxxxxxxxxxx> wrote: >> >> Hi, >> >>> On 21. Aug 2020, at 17:17, Himanshu Madhani <himanshu.madhani@xxxxxxxxxx> wrote: >>> >>> Hi Rene, >>> >>>> On Aug 21, 2020, at 9:47 AM, René Rebe <rene@xxxxxxxxxxxx> wrote: >>>> >>>> Hey, >>>> >>>>> On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@xxxxxxxxxx> wrote: >>>>> >>>>> Hi Rene, >>>>> >>>>> >>>>>> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@xxxxxxxxxxxxx> wrote: >>>>>> >>>>>> >>>>>> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >>>>>> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >>>>>> Unbreak by fixing endianess in nvram firmware defaults >>>>>> initialization. >>>>>> >>>>> >>>>> I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. >>>> >>>> Did I copy this wrong? >>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n_89mkIf$ >>>> >>> >>> Yes. The actual commit that change those lines is following >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98aee70d19a7e3203649fa2078464e4f402a0ad8__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXzu7NMDs$ >>> >>> So your fixes tag should be >>> >>> Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.”) >>> >>> And looking at the changes, I see that the frame_payload_size was changed to __le16 so this should not cause regression in your env. >>> >>> Interestingly this patch had fixed the regression and was originally tested on SunBlade-1000 >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4e08df3f91837656c36712f559d5ce8d80852760__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX5pZx4bu$ >>> >>> Can you try reverting 98aee70d19a7e and retest to confirm that this patch indeed introduced regression for you. >> >> Well, linux kernels since 3.17 did not work on this adapter on sparc64: >> >> and I reverted the whole 9a50aaefc1b896e734bf7faf3d085f71a360ce97 and factored the changes out and tested what I sent with 5..8.1. >> >> This was also reported back in the say, but somehow never applied, ..? >> >> https://urldefense.com/v3/__https://lore.kernel.org/lkml/86A17A977688E04689F40F6B1EC20EC601940E06A9@xxxxxxxxxxxxxxxx/__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX9w-_N_o$ >> > > Wow. This should have been done the moment Author acknowledged the revert. I am stumped that we are still talking about this revert after 5+ yrs and its not done. Would you please send just plain revert request for commit 98aee70d19a7e rather than reverting whole merge window patch. Well, how many people care about vintage Sun sparc64 hardware, ..? ;-) IMHO it makes no sense to revert this in full today. It does not revert cleanly and all the other __le16 conversions did not cause regressions that I know of. So instead having one not __le16 member, I suggest the partial revert I posted initially. I can resend it with the updated Fixes: reference as requested /* Firmware Initialization Control Block. */ - uint16_t version; + __le16 version; uint16_t reserved_1; ++<<<<<<< HEAD + __le16 frame_payload_size; + __le16 execution_throttle; + __le16 exchange_count; + __le16 hard_address; ++======= + uint16_t frame_payload_size; + uint16_t execution_throttle; + uint16_t exchange_count; + uint16_t hard_address; ++>>>>>>> parent of 98aee70d19a7... qla2xxx: Add endianizer to max_payload_size modifier. … @@@ -8411,11 -5903,11 +8437,19 @@@ qla81xx_nvram_config(scsi_qla_host_t *v * Set default initialization control block. */ memset(nv, 0, ha->nvram_size); ++<<<<<<< HEAD + nv->nvram_version = cpu_to_le16(ICB_VERSION); + nv->version = cpu_to_le16(ICB_VERSION); + nv->frame_payload_size = cpu_to_le16(2048); + nv->execution_throttle = cpu_to_le16(0xFFFF); + nv->exchange_count = cpu_to_le16(0); ++======= + nv->nvram_version = __constant_cpu_to_le16(ICB_VERSION); + nv->version = __constant_cpu_to_le16(ICB_VERSION); + nv->frame_payload_size = __constant_cpu_to_le16(2048); + nv->execution_throttle = __constant_cpu_to_le16(0xFFFF); + nv->exchange_count = __constant_cpu_to_le16(0); ++>>>>>>> parent of 98aee70d19a7... qla2xxx: Add endianizer to max_payload_size modifier. … >> Regards, >> René >> >>>>> Can you please provide details of the commit which introduced this regression. >>>> >>>> See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … >>>> >>>> Have a nice weekend, >>>> René >>>> >>>>> Also when you >>>>> resubmit this patch please use Fixes tag. See documentation here for the correct format. >>>>> >>>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n620C3iF$ >>>>> >>>>>> Signed-off-by: René Rebe <rene@xxxxxxxxxxxx> >>>>>> >>>>>> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >>>>>> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >>>>>> @@ -4603,18 +4603,18 @@ >>>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>>> nv->add_firmware_options[0] = BIT_5; >>>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>>> - nv->frame_payload_size = 2048; >>>>>> + nv->frame_payload_size = cpu_to_le16(2048); >>>>>> nv->special_options[1] = BIT_7; >>>>>> } else if (IS_QLA2200(ha)) { >>>>>> nv->firmware_options[0] = BIT_2 | BIT_1; >>>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>>> nv->add_firmware_options[0] = BIT_5; >>>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>>> - nv->frame_payload_size = 1024; >>>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>>> } else if (IS_QLA2100(ha)) { >>>>>> nv->firmware_options[0] = BIT_3 | BIT_1; >>>>>> nv->firmware_options[1] = BIT_5; >>>>>> - nv->frame_payload_size = 1024; >>>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>>> } >>>>>> >>>>>> nv->max_iocb_allocation = cpu_to_le16(256); >>>>>> >>>>>> -- >>>>>> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >>>>>> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ >>>>> >>>>> -- >>>>> Himanshu Madhani Oracle Linux Engineering >>>>> >>>> >>>> -- >>>> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-uIVWXr$ >>>> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n2dniKkK$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-v2wVJD$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n9g--eFu$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_nxllUxx0$ >>> >>> -- >>> Himanshu Madhani Oracle Linux Engineering >>> >> >> -- >> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6haBk6D$ >> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX4W9vLaa$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX04ZQeL4$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6DRH0bV$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXz0D-Wul$ > > -- > Himanshu Madhani Oracle Linux Engineering > -- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de
