Hi Mark,
Yes, I think this should be fine, you can add by reviewed-by at the take2.
Thanks and regards!
--------------
jack_wang
>Upon further investigation of the documentation, bit [2] (0x04) is the Force HDA mode, but the flashless operations are bit [3] (0x08) meaning Firmware to be loaded in HDA mode. I expect either SEEPROM bit set is enough.
>
>Xyratex is NOT setting the SEEPROM bits so the flags are 'incorrect' and a value of Zero, yet the hardware is in-fact in HDA-mode because of the lack of flash, so the mpi_uninit_check() still makes loads of sense to capture this state and engineer ourselves out of that problem.
>
>Jack, would the following trailing fragment to replace the six lines of the one you noted below resolve your concern? :
>
>+ return pm8001_ha->main_cfg_tbl.hda_mode_flag & 0xC;
>
>If so, I will 'take2' the patch with this change as I have not seen any other reviews.
>
>Thanks for the excellent catch.
>
>Sincerely -- Mark Salyzyn
>
>On Apr 30, 2012, at 9:49 AM, Mark Salyzyn wrote:
>
>> You are right ... however this code is 'never reached' since mpi_uninit_check() generally fails before this in the chip_in_hda_mode set of tests.
>>
>> I instrumented up, on our HDA mode chip, we are seeing a value of ZERO in MAIN_HDA_FLAGS_OFFSET ... Ahh, the world of initializing hardware that is in an indeterminate (code not running) state ...
>>
>> -- Mark
>>
>> On Apr 28, 2012, at 2:07 AM, Jack Wang wrote:
>>
>>>
>>> + data = pm8001_mr32(
>>> + pm8001_ha->main_cfg_tbl_addr,
>>> + pm8001_ha->main_cfg_tbl.hda_mode_flag);
>>> + PM8001_INIT_DBG(pm8001_ha,
>>> + pm8001_printk("*MAIN_HDA_FLAGS = 0x%x\n", data));
>>> + return data & 0x4;
>>>
>>> Here seems wrong, you may want to check the force hda bit? So you need to
>>> read offset #define MAIN_HDA_FLAGS_OFFSET 0x84/* DWORD 0x21 */
>>>
>>> Jack
>>>
>>>
>>>>
>>>> The pm8001 can be delivered as a standalone product with flash-programmed
>>>> firmware images, or without the flash present requiring the driver to
>>> upload
>>>> the images into the chip's RAM and then run. This is called HDA mode.
>>>>
>>>> We add support for this firmware upload in the enclosed patch. We try some
>>>> basic initialization checks of the Firmware, and if it appears dead, we
>>> make
>>>> the assumption the adapter must in-fact be halted in this HDA mode. The
>>>> Firmware images themselves have not been cleared for open-release by PMC,
>>> but
>>>> they are available in OpenSolaris <hint hint>. PMC's rationalization for
>>> not
>>>> wanting an open-release of the Firmware Images is that they do not want to
>>>> take support calls except from paying OEMs (such as Xyratex) that are
>>> embedding
>>>> PMC product into the motherboards and thus may have a tested combination
>>> of
>>>> Firmware and Hardware. Please respect this sentiment. Images are expected
>>> in:
>>>>
>>>> /lib/firmware/aap1img.bin
>>>> /lib/firmware/ilaimg.bin
>>>> /lib/firmware/iopimg.bin
>>>> /lib/firmware/istrimg.bin
>>>>
>>>> using the exact same naming convention as PMC and in OpenSolaris (and its
>>>> followon children) for these image files.
>>>>
>>>> Signed-off-by: Mark Salyzyn <mark_salyzyn@xxxxxxxxxxx>
>>>>
>>>> drivers/scsi/pm8001/pm8001_hwi.c | 584
>>>> +++++++++++++++++++++++++++++++++++---
>>>> drivers/scsi/pm8001/pm8001_hwi.h | 37 ++
>>>> drivers/scsi/pm8001/pm8001_init.c | 30 +
>>>> drivers/scsi/pm8001/pm8001_sas.h | 3
>>>> 4 files changed, 613 insertions(+), 41 deletions(-)
>>>>
>>>> Please see enclosed attachment
>>>
>>>
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@xxxxxxxxxxxxxxx
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>__________ Information from ESET NOD32 Antivirus, version of virus signature database 5659 (20101129) __________
>
>The message was checked by ESET NOD32 Antivirus.
>
>http://www.eset.com
>
>
>?韬{.n?????%??檩??w?{.n???{炳??Ф?塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
