Hi Stefan Please see my inline reply, thanks!
Yours, Ken -----Original Message----- From: Stefan Roese [mailto:s...@denx.de] Sent: 2017年4月5日 21:46 To: Ken Ma; Simon Glass Cc: u-boot@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:s...@denx.de] > Sent: 2017年4月3日 14:14 > To: Simon Glass; Ken Ma > Cc: u-boot@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 <m...@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@e0000 { >>> >>> compatible = "marvell,mvebu-scsi"; >>> >>> reg = <0xe0000 0x2000>; >>> >>> sata: sata@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot