Re: FireWire/SBP2 Target mode

From: Julian Calaby <julian.calaby@gmail.com>
To: Chris Boot <bootc@bootc.net>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>,
	Clemens Ladisch <clemens@ladisch.de>,
	target-devel@vger.kernel.org,
	linux1394-devel@lists.sourceforge.net,
	Boaz Harrosh <bharrosh@panasas.com>,
	Andy Grover <agrover@redhat.com>,
	linux-scsi@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>
Subject: Re: FireWire/SBP2 Target mode
Date: Tue, 7 Feb 2012 21:06:57 +1100	[thread overview]
Message-ID: <CAGRGNgXj37g6Q5W+hwkBjQHh3a8to+2Vq5Yx_omC8p9bdZrdBw@mail.gmail.com> (raw)
In-Reply-To: <4F30D4F6.1040802@bootc.net>

Hi,

On Tue, Feb 7, 2012 at 18:38, Chris Boot <bootc@bootc.net> wrote:
> On 06/02/2012 23:09, Chris Boot wrote:
>>
>>
>> On 6 Feb 2012, at 23:00, Julian Calaby wrote:
>>
>>> Hi,
>>>
>>> On Tue, Feb 7, 2012 at 09:28, Chris Boot<bootc@bootc.net>  wrote:
>>>>
>>>> On 6 Feb 2012, at 20:26, Stefan Richter wrote:
>>>>
>>>>> On Feb 06 Chris Boot wrote:
>>>>>>
>>>>>> On 06/02/2012 14:43, Clemens Ladisch wrote:
>>>>>>>
>>>>>>> Chris Boot wrote:
>>>>>>>>
>>>>>>>> You can pull the code from:
>>>>>>>> git://github.com/bootc/Linux-SBP-2-Target.git
>>>>>>>
>>>>>>>
>>>>>>> The TODO file says:
>>>>>>>>
>>>>>>>> * Update Juju so we can get the speed in the fw_address_handler
>>>>>>>> callback
>>>>>>>
>>>>>>>
>>>>>>> What is the speed needed for?
>>>>>>
>>>>>>
>>>>>> "The speed at which the block write request to the MANAGEMENT_AGENT
>>>>>> register is received shall determine the speed used by the target for
>>>>>> all subsequent requests to read the initiator’s configuration ROM,
>>>>>> fetch
>>>>>> ORB’s from initiator memory or store status at the initiator’s
>>>>>> status_FIFO. Command block ORB’s separately specify the speed for
>>>>>> requests addressed to the data buffer or page table."
>>>>>>
>>>>>> (T10/1155D Revision 4 page 53/54)
>>>>>
>>>>>
>>>>> I guess it is not too hard to add this to the AR-req handler.  On the
>>>>> other hand, I see little reason to follow the SBP-2 spec to the letter
>>>>> here.  The target driver could just use the maximum speed that the core
>>>>> figured out.  On the other hand, this requires of course
>>>>>  - the target to wait for core to finish scanning an initiator,
>>>>>  - the core to offer an API to look up an fw_device by a
>>>>>    card--generation--nodeID tuple.
>>>>>
>>>>> The intention of the spec is IMO clearly to enable target
>>>>> implementations
>>>>> that do not need to implement topology scanning.  I have a hard time to
>>>>> think of a valid scenario where an initiator needs to be able to steer
>>>>> a
>>>>> target towards a lower wire speed than what the participating links and
>>>>> PHYs actually support.
>>>>
>>>>
>>>> The only thing stopping me from getting the speed is the fact that
>>>> struct fw_request is opaque. The value is easily available from
>>>> request->response.speed and I kind of do that already in a very hackish way.
>>>> I've sent a separate patch which adds a function that can be used to access
>>>> that one value.
>>>>
>>>> Waiting until the bus scan is complete isn't actually that great as I
>>>> see the first LOGIN requests often before the fw_node is seen at all. I'd
>>>> have to turn away the requester and hope they try again. I'm fairly sure my
>>>> little tweak in my patch is a simple enough solution.
>>>
>>>
>>> Stupid question: Could you use a completion queue or something
>>> equivalent to wait until you have seen the fw_node, *then* process the
>>> LOGIN request?
>>
>>
>> The fw_address_handler callback is called in interrupt context, and I
>> can't sleep from within there. As far as I'm aware I must call
>> fw_send_response() from within the callback and can't defer that until I've
>> scheduled something on a work queue. Please correct me if I'm wrong though,
>> as that might be useful anyway.
>
>
> Hmm sorry I've thought about this overnight and clearly I was talking
> rubbish. Yes, I need to reply in the fw_address_handler but all I tend to do
> in there is schedule a task to the the main part of the work anyway. As most
> of the operations require fetching an ORB from the initiator I have to do
> this from user context.
>
> So it's possible I could do this by waiting in my scheduled work function
> until the fw_node is available and get the speed from that - but that seems
> like an inordinate amount of work when I can follow the standard and do it
> really easily by pulling it out of the fw_request.

Fair enough, I assumed that there might have been some reason why you
wouldn't have it at that point, not just convention getting in the
way.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/