Re: Re: [PATCH] pm8001: support HDA (flashless) mode

[jack_wang <jack_wang@usish.com>]

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@xyratex.com>
>>>> 
>>>> 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@vger.kernel.org
>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
>
>
>