Re: [PATCH] Add iSCSI iBFT support.
From: Randy Dunlap
Date: Thu Sep 27 2007 - 00:32:40 EST
On Wed, 26 Sep 2007 20:52:43 -0400 Konrad Rzeszutek wrote:
> > > +config ISCSI_IBFT
> > > + tristate "iSCSI Boot Firmware Table Attributes"
> > > + depends on X86
> > why only on X86?
> PowerPC exports this data via the OpenFirmware so it already shows in
> the /sysfs entries. I was thinking to combine those sysfs entries under this
> code, but that is something in the future.
> In regards to all other platforms, I figured I would only make it supported
> under platforms that have been tested. Is there anything that stops this from
> working for example of IA64? Well no. The hardware that supports the iBFT is
> either in the BIOS or in NICs - so the SGI or HP boxes _should_ work, however
> I am not comfortable to make it supported unless I've tested it.
That's not how Linux development works. You (we) have a huge
test lab around the world. You practice "release early, release
often" and get testing/feedback on it. Maybe even patches. :)
> > Need blank line here... except why is this function in the header
> Fixed blank line.
> > file? and why is it inline?
> Q: "Why is this function in the header file"
> If the find_ibft() was to be implemented in firmware/iscsi_ibft.c code the
> firmware-driver couldn't be compiled as a module (or rather it could, but
> when the vmlinuz was to be linked it would complain about missing symbol -
> find_ibft). I was thinking to let the user give the choice whether they want
> to load this firmware driver or have it built-in the kernel.
> Q:"Why is it inline"
> Uhh. It does not need to. I will remove the 'inline' part.
But we strongly prefer not to have non-inline C code in header files,
[and the function does not need to be inline]
so find_ibft() needs a home in some source file/code that won't be built
as a loadable module, right? And preferably not duplicated (i386 &
x86_64 versions; but we should see a merged x86/ arch soon, so it
sounds). Would ia64 have its own version of find_ibft() or use this
I think that for now you can put find_ibft() in both setup.c files
and the merged x86/ arch tree can eliminate one of them.
On looking back at the patch, why aren't the ibft_phys and find_ibft()
parts of both setup.c patches surrounded by #ifdef CONFIG_ISCSI_IBFT &
Phaedrus says that Quality is about caring.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/