All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Salyzyn <mark_salyzyn@xyratex.com>
To: Jack Wang <jack_wang@usish.com>
Cc: "Mark Salyzyn" <mark_salyzyn@xyratex.com>,
	linux-scsi@vger.kernel.org,
	"'James Bottomley'" <JBottomley@parallels.com>,
	'lindar_liu' <lindar_liu@usish.com>, '于爱华' <crystal_yu@usish.com>,
	'john_gong' <john_gong@usish.com>
Subject: Re: [PATCH] pm8001: support HDA (flashless) mode
Date: Mon, 30 Apr 2012 12:56:30 -0400	[thread overview]
Message-ID: <E53424C2-2CAE-43AA-81EE-0E7B44586C6F@xyratex.com> (raw)
In-Reply-To: <73464C75-47D9-4409-87C2-8DDD0CBD5C67@xyratex.com>

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
>> 
>> 
> 


  reply	other threads:[~2012-04-30 16:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-27 17:10 [PATCH] pm8001: support HDA (flashless) mode Mark Salyzyn
2012-04-28  1:06 ` Jack Wang
2012-04-30 14:00   ` Mark Salyzyn
2012-04-28  4:44 ` Re " Jack Wang
2012-04-30 13:43   ` Mark Salyzyn
2012-04-28  6:07 ` Jack Wang
2012-04-30 13:49   ` Mark Salyzyn
2012-04-30 16:56     ` Mark Salyzyn [this message]
2012-05-01  1:10       ` jack_wang
2012-05-01 12:45 ` [PATCH] pm8001: support HDA (flashless) mode (take 2) Mark Salyzyn
2012-05-03  0:35   ` RE " Jack Wang
2012-05-03 17:18     ` Mark Salyzyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E53424C2-2CAE-43AA-81EE-0E7B44586C6F@xyratex.com \
    --to=mark_salyzyn@xyratex.com \
    --cc=JBottomley@parallels.com \
    --cc=crystal_yu@usish.com \
    --cc=jack_wang@usish.com \
    --cc=john_gong@usish.com \
    --cc=lindar_liu@usish.com \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.