[U-Boot] [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node

[Stefan Roese]

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.

> 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