[U-Boot] [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node
Ken Ma
make at marvell.com
Thu Apr 6 01:32:31 UTC 2017
Hi Stefan
Please see my inline reply, thanks!
Yours,
Ken
-----Original Message-----
From: Stefan Roese [mailto:sr at denx.de]
Sent: 2017年4月5日 21:46
To: Ken Ma; Simon Glass
Cc: u-boot at lists.denx.de; Michal Simek; Kostya Porotchkin; Hua Jing; Wilson Ding
Subject: Re: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node
Hi Ken,
On 05.04.2017 11:29, Ken Ma wrote:
> Hi Stefan, Hi Simon
>
> Please see my inline reply, thanks!
>
> Yours,
> Ken
>
> -----Original Message-----
> From: Stefan Roese [mailto:sr at denx.de]
> Sent: 2017年4月3日 14:14
> To: Simon Glass; Ken Ma
> Cc: u-boot at lists.denx.de; Michal Simek; Kostya Porotchkin; Hua Jing;
> Wilson Ding
> Subject: Re: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node
>
> Hi Simon, Hi Ken,
>
> On 01.04.2017 06:22, Simon Glass wrote:
>> On 27 March 2017 at 02:28, Ken Ma <make at marvell.com> wrote:
>>> Hi Stefan
>>>
>>>
>>>
>>> Thanks a lot for your kind reply.
>>>
>>>
>>>
>>> But I still do not think it's very good to change sata's uclass id
>>> from "UCLASS_AHCI" to "UCLASS_SCSI".
>>>
>>> If we do such change, UCLASS_AHCI is lost since from the sata.c
>>> codes, it does the AHCI initialization work but not SCSI initialization work.
>>>
>>> If u-boot supports ISIS scanner which supports SCSI, I think its
>>> uclass id should be like UCLASS_ISIS but not UCLASS_SCSI.
>>>
>>> And if we set sata's uclass id as "UCLASS_SCSI", it should provide
>>> basic SCSI function, then why can’t we connect a parallel SCSI
>>> device like SCSI scanner or cd-rom to the SATA interface?
>>>
>>> From https://en.wikipedia.org/wiki/Serial_ATA#SATA_and_SCSI
>>>
>>> In general, SATA devices link compatibly to SAS enclosures and
>>> adapters, whereas SCSI devices cannot be directly connected to a
>>> SATA bus
>>>
>>>
>>>
>>>
>>>
>>> Actually Marvell’s sata controller is SAS(Serial Attached SCSI
>>> system), it integrates SCSI and SATA(AHCI); SAS provides a SCSI bus
>>> which works only in SAS range(for example, 2 sata ports in SAS), so
>>> actually the SCSI bus controller is not "virtual" controllers but
>>> has the same device base register as SATA.
>>>
>>>
>>>
>>> From https://en.wikipedia.org/wiki/Serial_Attached_SCSI
>>>
>>> A typical Serial Attached SCSI system consists of the following
>>> basic
>>> components:
>>>
>>> 1. An initiator: a device that originates device-service and
>>> task-management requests for processing by a target device and
>>> receives responses for the same requests from other target devices.
>>> Initiators may be provided as an on-board component on the
>>> motherboard (as is the case with many server-oriented motherboards) or as an add-on host bus adapter.
>>>
>>> 2. A target: …
>>>
>>> So in my opinion, there are two ways to implement SAS as below
>>>
>>> A. If our codes provide SAS controller as an on-board component –
>>> then a uclass id of UCLASS_SAS should be defined and then in
>>> scsi_scan() of scsi_scan.c, both devices of UCLASS_SCSI and UCLASS_SAS should be scanned.
>>> In such implementation, UCLASS_SCSI is for parallel SCSI while
>>> UCLASS_SAS is for serial attached SCSI;
>>>
>>> B. SAS works as an add-on host bus adapter as above said, SAS’s
>>> SCSI controller and AHCI controller are both itself as below - SCSI
>>> controller is not a virtual device, it exists and only works in SAS
>>> internal range(since there is no UCLASS_SAS, I take this way);
>>>
>>> Although the SAS’s SCSI controller does not need to any special
>>> hardware configuration; but actually I think there is something to
>>> do, we should bind special scsi_exec() to SCSI devices in SCSI
>>> driver or SAS driver (For different SCSI controls, SAS must have
>>> different implementation of
>>> scsi_exec() comparing to SCSI scanner, or other SCSI devices)
>>>
>>> By the way, I think we should move the work of creating block
>>> devices to scsi-uclass.c
>>>
>>> scsi: scsi at e0000 {
>>>
>>> compatible = "marvell,mvebu-scsi";
>>>
>>> reg = <0xe0000 0x2000>;
>>>
>>> sata: sata at e0000 {
>>>
>>> compatible =
>>> "marvell,armada-3700-ahci";
>>>
>>> reg = <0xe0000 0x2000>;
>>>
>>> interrupts = <GIC_SPI 27
>>> IRQ_TYPE_LEVEL_HIGH>;
>>>
>>> };
>>
>> Does this match the Linux DT?
>
> No, and this is my main concern. This patch series introduces a new
> "scsi" DT node and moves the controller(s) one level up into this
> newly created DT node (Ken please correct me if I'm wrong here).
> We simply can't make such changes here in U-Boot without this being
> first discussed and decided on the Linux DT mailing list with the DT
> maintainers.
>
> Ken, how is this problem solved / handled in Linux without this DT
> changes up until now?
>
> [Ken] Actually I do not find any SCSI nodes/compatible strings In
> Linux; if aligned to linux, then we should have no scsi nodes at all
> in u-boot.
Thats exactly what I meant. The DT represents the hardware and only controllers / devices etc are listed here. As such, only the SATA, AHCI, IDE (etc.) controllers are listed behind their internal SoC / CPU busses in the DT. Unfortunately we can't come up with this new SCSI DT node for U-Boot and move all the controllers into this node, as we need to try to stay in sync with the Linux DT, which is the reference.
[Ken] If aligned to linux, we should not add the class id of "UCLASS_SCSI" but add a middle scsi layer. We can register to scsi in scsi device driver(like sata.c) after adding some middle scsi layer, but we should not use UCLASS_SCSI to replace UCLASS_AHCI directly.
Now in u-boot scsi_scan() uses UCLASS_SCSI objects as scsi bus actually, if there are 2 different scsi devices in one same scsi bus and we classify the 2 different scsi devices into UCLASS_SCSI, then scsi_scan() will regard as there are 2 scsi buses but not one. We should not classify scsi devices into UCLASS_SCSI.
In my understanding, since in u-boot UCLASS_SCSI is for SCSI host controller(bus function), we should have such a device bus node.
Linux SCSI freamework
Upper level: Disk driver(sd) tape driver(st) CDROM driver(sr) Generic driver(sg)
Middle level: common service layer
Lower level: Fibre channel driver SAS driver iSCSI driver ... Bridge driver
> I am not familiar with Linux SCSI implementation, it seems that SCSI
> bus is implemented as a middle layer in Linux since it has no SCSI fdt
> nodes.
Frankly, I've not looked at SCSI in Linux for quite some time.
So I can't really tell you how this is handled there. But I'm pretty sure that no SCSI DT nodes / properties are involved here.
Thanks,
Stefan
More information about the U-Boot
mailing list