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

2017-04-09 Thread Simon Glass 
Hi Ken,

On 5 April 2017 at 19:32, Ken Ma  wrote:
> 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  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@e {

 compatible = "marvell,mvebu-scsi";

 reg = <0xe 0x2000>;

 sata: sata@e {

   compatible =
 "marvell,armada-3700-ahci";

   reg = <0xe 0x2000>;

   interrupts = >>> 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

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

2017-04-05 Thread Ken Ma 
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  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@e {
>>>
>>> compatible = "marvell,mvebu-scsi";
>>>
>>> reg = <0xe 0x2000>;
>>>
>>> sata: sata@e {
>>>
>>>   compatible = 
>>> "marvell,armada-3700-ahci";
>>>
>>>   reg = <0xe 0x2000>;
>>>
>>>   interrupts = >> 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

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

2017-04-05 Thread 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: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  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@e {

compatible = "marvell,mvebu-scsi";

reg = <0xe 0x2000>;

sata: sata@e {

  compatible =
"marvell,armada-3700-ahci";

  reg = <0xe 0x2000>;

  interrupts = ;

};


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

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

2017-04-05 Thread Ken Ma 
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  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@e {
>>
>> compatible = "marvell,mvebu-scsi";
>>
>> reg = <0xe 0x2000>;
>>
>> sata: sata@e {
>>
>>   compatible = 
>> "marvell,armada-3700-ahci";
>>
>>   reg = <0xe 0x2000>;
>>
>>   interrupts = > 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.
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.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-04-03 Thread Stefan Roese 

Hi Simon, Hi Ken,

On 01.04.2017 06:22, Simon Glass wrote:

On 27 March 2017 at 02:28, Ken Ma  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@e {

compatible = "marvell,mvebu-scsi";

reg = <0xe 0x2000>;

sata: sata@e {

  compatible = "marvell,armada-3700-ahci";

  reg = <0xe 0x2000>;

  interrupts = ;

};


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?

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-03-31 Thread Simon Glass 
Hi Ken,

On 27 March 2017 at 02:28, Ken Ma  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@e {
>
> compatible = "marvell,mvebu-scsi";
>
> reg = <0xe 0x2000>;
>
> sata: sata@e {
>
>   compatible = "marvell,armada-3700-ahci";
>
>   reg = <0xe 0x2000>;
>
>   interrupts = ;
>
> };

Does this match the Linux DT?
>
>   };
>
>
>
>
>
> Yours,
>
> Ken

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-03-27 Thread Ken Ma 
Hi Stefan

I think it's a good way, but I wonder why the codes calls ffs() but not fls()?
If the linkmap is 0x, it seems that ffs() returns 1 while fls() returns 16, 
I think max_id should be 16 then.

Yours,
Ken

-Original Message-
From: Stefan Roese [mailto:s...@denx.de] 
Sent: 2017年3月24日 21:24
To: Ken Ma; u-boot@lists.denx.de
Cc: Simon Glass; Michal Simek; Kostya Porotchkin; Hua Jing; Wilson Ding
Subject: Re: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node

Hi Ken,

btw, regarding the max-lun and max-id you could perhaps take a look at this 
recent patch, also dealing with these parameters:

https://patchwork.ozlabs.org/patch/743160/

I've not looked too close into this, just wanted to point it out to you.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-03-27 Thread Ken Ma 
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@e {

compatible = "marvell,mvebu-scsi";

reg = <0xe 0x2000>;

sata: sata@e {

  compatible = "marvell,armada-3700-ahci";

  reg = <0xe 0x2000>;

  interrupts = ;

};

  };





Yours,

Ken





-Original Message-
From: Stefan Roese [mailto:s...@denx.de]
Sent: 2017年3月24日 21:22
To: Ken Ma; u-boot@lists.denx.de
Cc: Simon Glass; Michal Simek; Kostya Porotchkin; Hua Jing; Wilson Ding
Subject: Re: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node



Hi Ken,



On 24.03.2017 05:11, Ken Ma wrote:







>> b/arch/arm/dts/armada-3720-db.dts index 85761af..9fc60f6 100644

>

>> --- a/arch/arm/dts/armada-3720-db.dts

>

>> +++ b/arch/arm/dts/armada-3720-db.dts

>

>> @@ -89,6 +89,10 @@

>

>> status = "okay";

>

>>  };

>

>>

>

>> + {

>

>> +   status = "okay";

>

>> +};

>

>> +

>

>>  /* CON3 */

>

>>   {

>

>> status = "okay";

>

>> diff --git a/arch/arm/dts/armada-37xx.dtsi

>

>> b/arch/arm/dts/armada-37xx.dtsi index 062f2a6..de5d3a1 100644

>

>> --- a/arch/arm/dts/armada-37xx.dtsi

>

>> +++ b/arch/arm/dts/armada-37xx.dtsi

>

>> @@ -149,11 +149,19 @@

>

>>   status = "disabled";

>

>> };

>

>>

>

>> -   sata: sata@e {

>

>> - compatible = "marvell,armada-3700-ahci";

>

>> - reg = <0xe 0x2000>;

>

>> - interrupts = ;

>

>> +   scsi: scsi {

>

>> + compatible = "marvell,mvebu-scsi";

>

>> + #address-cells = <1>;

>

>> + #size-cells = <1>;

>

>> + max-id = <1>;

>

>> + max-lun = <1>;

>

>>   status = "disabled";

>

>> + sata: sata@e {

>

>> +   compatible = "marvell,armada-3700-ahci";

>

>> +   reg =

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

2017-03-24 Thread Stefan Roese 

Hi Ken,

btw, regarding the max-lun and max-id you could perhaps take
a look at this recent patch, also dealing with these parameters:

https://patchwork.ozlabs.org/patch/743160/

I've not looked too close into this, just wanted to point it
out to you.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-03-24 Thread Stefan Roese 

Hi Ken,

On 24.03.2017 05:11, Ken Ma wrote:




b/arch/arm/dts/armada-3720-db.dts index 85761af..9fc60f6 100644



--- a/arch/arm/dts/armada-3720-db.dts



+++ b/arch/arm/dts/armada-3720-db.dts



@@ -89,6 +89,10 @@



status = "okay";



 };







+ {



+   status = "okay";



+};



+



 /* CON3 */



  {



status = "okay";



diff --git a/arch/arm/dts/armada-37xx.dtsi



b/arch/arm/dts/armada-37xx.dtsi index 062f2a6..de5d3a1 100644



--- a/arch/arm/dts/armada-37xx.dtsi



+++ b/arch/arm/dts/armada-37xx.dtsi



@@ -149,11 +149,19 @@



  status = "disabled";



};







-   sata: sata@e {



- compatible = "marvell,armada-3700-ahci";



- reg = <0xe 0x2000>;



- interrupts = ;



+   scsi: scsi {



+ compatible = "marvell,mvebu-scsi";



+ #address-cells = <1>;



+ #size-cells = <1>;



+ max-id = <1>;



+ max-lun = <1>;



  status = "disabled";



+ sata: sata@e {



+   compatible = "marvell,armada-3700-ahci";



+   reg = <0xe 0x2000>;



+   interrupts = ;



+   status = "disabled";



+ };



};







gic: interrupt-controller@1d0 {








I see that you introduce a "scsi" DT node and move the SATA controller
one "level up". I'm not sure if such a change is acceptable as we try to
re-use the DT from Linux. Or thinking more about this, I'm pretty sure
that such a change is not acceptable in general.



Can't you use the existing DT layout and use the
"marvell,armada-3700-ahci" (and other perhaps?) compatible property
instead for driver probing? Not sure how to handle the "max-id" and
"max-lun" properties though. We definitely can't just add some ad-hoc
properties here in U-Boot which have no chance for Linux upstream
acceptance.



[Ken] Because scsi is a bus, for example, if there are 2 scsi buses,
each bus has some scsi device controllers connected as below.


Scsi ID 0 Scsi ID 1 Scsi ID 2 Scsi ID 3



HDD0  HDD1   tape0  cd-rom0

||||||||

===

SCSI BUS1



HDD2  HDD3   tape1  cd-rom2

||||||||

===

SCSI BUS2



As far as I understand, you are looking at this from the external point
of view (SCSI devices connected to the board). But the DT describes
the hardware / interfaces from the CPU / SoC point of view. The
SCSI bus topology can change, depending on which and how the "user"
connects the multiple SCSI devices to the different controllers.
This is definitely not what we can describe in the DT for the
board. Here only the view of the internal controllers / interfaces
is described (UART controller at 0x..., SPI controller at 0x..,
AHCI controller at 0x..., etc).


Then in my opinion, since now scsi has its own class id and its
compatible string, then the scsi device controllers dts node should be
above the scsi node.


As mentioned before, we are not "free" to insert "virtual" controllers
in the DT. Only real hardware interfaces can be described.


If we keep existing DT layout and keep "marvell,armada-3700-ahci"’s
uclass id as UCLASS_AHCI(there are no scsi nodes but only ahci nodes),
then scsi_scan() can not find a3700’s sata at all since there are no
UCLASS_SCSI devices;


I've attached the small patch that I've send you a few weeks ago.
Didn't this work at all? IIRC, then the devices connected to the
SATA ports cuold be detected this way.


If we keep existing DT layout and set scsi devices’ uclass id to be
UCLASS_SCSI, how can we know that hdd0 and hdd1 are in scsi bus1 but
hdd2 and hdd3 are in scsi bus2?  For each scsi bus, its max id should be
4; but now how to set each scsi device’ scsi id?


Not sure if I understand you correctly here. Could you please
describe the problem that you are facing, using an example? That
would be clearer, at least to me.


So I think we should move scsi devices “level up” on the scsio bus.


Might be that I misunderstand you, but I think you are still
viewing this scenario from the external point of view and not
the internal one (as mentioned before). This is not, what the
DT is supposed to describe though.

Thanks,
Stefan
>From 0c071f4815ad76ed9eb64e4261066856062ba115 Mon Sep 17 00:00:00 2001
From: Stefan Roese 
Date: Fri, 13 Jan 2017 12:53:20 +0100
Subject: [PATCH] ahci: Add DM based support for the Marvell MVEBU SATA
 controller

This

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

2017-03-23 Thread Ken Ma 
+ Hua, Wilson

From: Ken Ma
Sent: 2017年3月24日 11:04
To: 'Stefan Roese'; u-boot@lists.denx.de
Cc: Simon Glass; Michal Simek; Kostya Porotchkin
Subject: RE: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node


Hi Stefan



Thanks a lot for your kind advice and help!

Please see my reply inline.



Yours,

Ken



-Original Message-
From: Stefan Roese [mailto:s...@denx.de]
Sent: 2017年3月23日 22:06
To: Ken Ma; u-boot@lists.denx.de
Cc: Simon Glass; Michal Simek
Subject: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node



External Email



--

Hi Ken,



On 23.03.2017 10:29, m...@marvell.com wrote:

> From: Ken Ma >

>

> - Add scsi node which acts as a bus for scsi devices, armada3700 has

>   only 1 scsi interface, so max-id is 1, and the logic unit number is

>   also 1 for armada3700;

> - Since a3700's scsi is sas(serial attached scsi) which is compatible

>   for sata and sata hard disk is a sas device, so move sata node to be

>   under scsi node.

>

> Signed-off-by: Ken Ma >

> Cc: Simon Glass >

> Cc: Stefan Roese >

> Cc: Michal Simek >

> Reviewed-on: http://vgitil04.il.marvell.com:8080/35303

> Tested-by: iSoC Platform CI >

> Reviewed-by: Kostya Porotchkin >

> Reviewed-by: Omri Itach >

> ---

>  arch/arm/dts/armada-3720-db.dts |  4 

>  arch/arm/dts/armada-37xx.dtsi   | 16 

>  2 files changed, 16 insertions(+), 4 deletions(-)

>

> diff --git a/arch/arm/dts/armada-3720-db.dts

> b/arch/arm/dts/armada-3720-db.dts index 85761af..9fc60f6 100644

> --- a/arch/arm/dts/armada-3720-db.dts

> +++ b/arch/arm/dts/armada-3720-db.dts

> @@ -89,6 +89,10 @@

> status = "okay";

>  };

>

> + {

> +   status = "okay";

> +};

> +

>  /* CON3 */

>   {

> status = "okay";

> diff --git a/arch/arm/dts/armada-37xx.dtsi

> b/arch/arm/dts/armada-37xx.dtsi index 062f2a6..de5d3a1 100644

> --- a/arch/arm/dts/armada-37xx.dtsi

> +++ b/arch/arm/dts/armada-37xx.dtsi

> @@ -149,11 +149,19 @@

>   status = "disabled";

> };

>

> -   sata: sata@e {

> - compatible = "marvell,armada-3700-ahci";

> - reg = <0xe 0x2000>;

> - interrupts = ;

> +   scsi: scsi {

> + compatible = "marvell,mvebu-scsi";

> + #address-cells = <1>;

> + #size-cells = <1>;

> + max-id = <1>;

> + max-lun = <1>;

>   status = "disabled";

> + sata: sata@e {

> +   compatible = "marvell,armada-3700-ahci";

> +   reg = <0xe 0x2000>;

> +   interrupts = ;

> +   status = "disabled";

> + };

> };

>

> gic: interrupt-controller@1d0 {

>



I see that you introduce a "scsi" DT node and move the SATA controller one 
"level up". I'm not sure if such a change is acceptable as we try to re-use the 
DT from Linux. Or thinking more about this, I'm pretty sure that such a change 
is not acceptable in general.



Can't you use the existing DT layout and use the "marvell,armada-3700-ahci" 
(and other perhaps?) compatible property instead for driver probing? Not sure 
how to handle the "max-id" and "max-lun" properties though. We definitely can't 
just add some ad-hoc properties here in U-Boot which have no chance for Linux 
upstream acceptance.



[Ken] Because scsi is a bus, for example, if there are 2 scsi buses, each bus 
has some scsi device controllers connected as below.



Scsi ID 0 Scsi ID 1 Scsi ID 2 Scsi ID 3



HDD0  HDD1   tape0  cd-rom0

||||||||

===

SCSI BUS1



HDD2  HDD3   tape1  cd-rom2

||||||||

===

SCSI BUS2





Then in my opinion, since now scsi has its own class id and its compatible 
string, then the scsi device controllers dts node should be above the scsi node.

If we keep existing DT layout and keep "marvell,armada-3700-ahci"’s uclass id 
as UCLASS_AHCI(there are no scsi nodes but only ahci nodes), then scsi_scan() 
can not find a3700’s sata at

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

2017-03-23 Thread Ken Ma 
Hi Stefan



Thanks a lot for your kind advice and help!

Please see my reply inline.



Yours,

Ken



-Original Message-
From: Stefan Roese [mailto:s...@denx.de]
Sent: 2017年3月23日 22:06
To: Ken Ma; u-boot@lists.denx.de
Cc: Simon Glass; Michal Simek
Subject: [EXT] Re: [PATCH 7/7] scsi: dts: a3700: add scsi node



External Email



--

Hi Ken,



On 23.03.2017 10:29, m...@marvell.com wrote:

> From: Ken Ma >

>

> - Add scsi node which acts as a bus for scsi devices, armada3700 has

>   only 1 scsi interface, so max-id is 1, and the logic unit number is

>   also 1 for armada3700;

> - Since a3700's scsi is sas(serial attached scsi) which is compatible

>   for sata and sata hard disk is a sas device, so move sata node to be

>   under scsi node.

>

> Signed-off-by: Ken Ma >

> Cc: Simon Glass >

> Cc: Stefan Roese >

> Cc: Michal Simek >

> Reviewed-on: http://vgitil04.il.marvell.com:8080/35303

> Tested-by: iSoC Platform CI >

> Reviewed-by: Kostya Porotchkin >

> Reviewed-by: Omri Itach >

> ---

>  arch/arm/dts/armada-3720-db.dts |  4 

>  arch/arm/dts/armada-37xx.dtsi   | 16 

>  2 files changed, 16 insertions(+), 4 deletions(-)

>

> diff --git a/arch/arm/dts/armada-3720-db.dts

> b/arch/arm/dts/armada-3720-db.dts index 85761af..9fc60f6 100644

> --- a/arch/arm/dts/armada-3720-db.dts

> +++ b/arch/arm/dts/armada-3720-db.dts

> @@ -89,6 +89,10 @@

> status = "okay";

>  };

>

> + {

> +   status = "okay";

> +};

> +

>  /* CON3 */

>   {

> status = "okay";

> diff --git a/arch/arm/dts/armada-37xx.dtsi

> b/arch/arm/dts/armada-37xx.dtsi index 062f2a6..de5d3a1 100644

> --- a/arch/arm/dts/armada-37xx.dtsi

> +++ b/arch/arm/dts/armada-37xx.dtsi

> @@ -149,11 +149,19 @@

>   status = "disabled";

> };

>

> -   sata: sata@e {

> - compatible = "marvell,armada-3700-ahci";

> - reg = <0xe 0x2000>;

> - interrupts = ;

> +   scsi: scsi {

> + compatible = "marvell,mvebu-scsi";

> + #address-cells = <1>;

> + #size-cells = <1>;

> + max-id = <1>;

> + max-lun = <1>;

>   status = "disabled";

> + sata: sata@e {

> +   compatible = "marvell,armada-3700-ahci";

> +   reg = <0xe 0x2000>;

> +   interrupts = ;

> +   status = "disabled";

> + };

> };

>

> gic: interrupt-controller@1d0 {

>



I see that you introduce a "scsi" DT node and move the SATA controller one 
"level up". I'm not sure if such a change is acceptable as we try to re-use the 
DT from Linux. Or thinking more about this, I'm pretty sure that such a change 
is not acceptable in general.



Can't you use the existing DT layout and use the "marvell,armada-3700-ahci" 
(and other perhaps?) compatible property instead for driver probing? Not sure 
how to handle the "max-id" and "max-lun" properties though. We definitely can't 
just add some ad-hoc properties here in U-Boot which have no chance for Linux 
upstream acceptance.



[Ken] Because scsi is a bus, for example, if there are 2 scsi buses, each bus 
has some scsi device controllers connected as below.



Scsi ID 0 Scsi ID 1 Scsi ID 2 Scsi ID 3



HDD0  HDD1   tape0  cd-rom0

||||||||

===

SCSI BUS1



HDD2  HDD3   tape1  cd-rom2

||||||||

===

SCSI BUS2





Then in my opinion, since now scsi has its own class id and its compatible 
string, then the scsi device controllers dts node should be above the scsi node.

If we keep existing DT layout and keep "marvell,armada-3700-ahci"’s uclass id 
as UCLASS_AHCI(there are no scsi nodes but only ahci nodes), then scsi_scan() 
can not find a3700’s sata at all since there are no UCLASS_SCSI devices;



If we keep existing DT layout and set scsi devices’ uclass id to be 
UCLASS_SCSI, how can we know that hdd0 and hdd1 are in scsi bus1 but hdd2 and 
hdd3 are in scsi bus2?  For each scsi bus,